Re: [Spice-devel] [PATCH spice-common 1/4] codegen: Use has_end_attr instead of has_attr("end")
On 8/13/19 7:56 PM, Frediano Ziglio wrote:
Just style, they do the same thing, but is more coherent
with the rest of the code.
Signed-off-by: Frediano Ziglio
Ack.
Uri.
---
python_modules/demarshal.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index d3147b7..6d8dbdd 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -804,7 +804,7 @@ def write_array_parser(writer, member, nelements, array,
dest, scope):
element_type = array.element_type
if member:
array_start = dest.get_ref(member.name)
-at_end = member.has_attr("end")
+at_end = member.has_end_attr()
else:
array_start = "end"
at_end = True
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common 3/4] codegen: Exit with error on error generating C structures
On 8/13/19 7:56 PM, Frediano Ziglio wrote: This was some left-over during development of C structure generations (the single structure generation was skipped). Signed-off-by: Frediano Ziglio Ack. Uri. --- spice_codegen.py | 1 + 1 file changed, 1 insertion(+) diff --git a/spice_codegen.py b/spice_codegen.py index 0532d6f..d3a1bf5 100755 --- a/spice_codegen.py +++ b/spice_codegen.py @@ -283,6 +283,7 @@ current: print >> sys.stderr, 'type %s' % t print >> sys.stderr, writer.getvalue() traceback.print_exc(sys.stderr) +sys.exit(1) def generate_declarations(): writer = codegen.CodeWriter() ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-common PATCH] test-marshallers.proto: ArrayMessage: make space for name
>
> Do it by adding @end tag.
> Without it the allocated memory has no space for 'name'.
>
> Also fix SpiceMsgMainArrayMessage tests/test-marshallers.h,
> replacing int8_t* name with int8_t name[].
name[0] ?
> This makes name an "in-structure" array with no pre-defined size
> instead of a pointer.
> The size is defined by the message size.
>
> Signed-off-by: Uri Lublin
Otherwise,
Acked-by: Frediano Ziglio
I tested and the change does not affect the initial reason for this type
declaration.
> ---
>
> Since v1:
>- fix 'name' in tests/test-marshallers.h too
>- more information in the commit log
>
> ---
> tests/test-marshallers.h | 2 +-
> tests/test-marshallers.proto | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/test-marshallers.h b/tests/test-marshallers.h
> index 7686067..8ca736e 100644
> --- a/tests/test-marshallers.h
> +++ b/tests/test-marshallers.h
> @@ -10,7 +10,7 @@ typedef struct {
> } SpiceMsgMainShortDataSubMarshall;
>
> typedef struct {
> -int8_t *name;
> +int8_t name[0];
> } SpiceMsgMainArrayMessage;
>
> typedef struct {
> diff --git a/tests/test-marshallers.proto b/tests/test-marshallers.proto
> index 34cc892..eabd487 100644
> --- a/tests/test-marshallers.proto
> +++ b/tests/test-marshallers.proto
> @@ -6,7 +6,7 @@ channel TestChannel {
> } ShortDataSubMarshall;
>
> message {
> - int8 name[];
> + int8 name[] @end;
> } ArrayMessage;
>
> message {
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-common 4/4] codegen: Check validity of array members
Check that combination of fields for an array does not
lead to unsafe code.
check_valid method came from generate_c_declaration with
some more check and it's use in demarshaller to validate
the array if the structure is not generated.
Signed-off-by: Frediano Ziglio
---
python_modules/demarshal.py | 2 ++
python_modules/ptypes.py| 18 +-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index acd4b6f..3736976 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -315,6 +315,8 @@ def write_validate_pointer_item(writer, container, item,
scope, parent_scope, st
def write_validate_array_item(writer, container, item, scope, parent_scope,
start,
want_nw_size, want_mem_size, want_extra_size):
array = item.type
+if item.member:
+array.check_valid(item.member)
is_byte_size = False
element_type = array.element_type
if array.is_bytes_length():
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 311ce3d..7a23bca 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -485,7 +485,23 @@ class ArrayType(Type):
def c_type(self):
return self.element_type.c_type()
+def check_valid(self, member):
+if member.has_attr("chunk") or member.has_attr("as_ptr"):
+return
+if member.has_attr("to_ptr") or member.has_attr("ptr_array"):
+if not (self.is_identifier_length() or self.is_constant_length()):
+raise Exception("Unsecure, no length of array")
+return
+if member.has_end_attr():
+return
+if self.is_remaining_length():
+raise Exception('C output array is not allocated')
+if self.is_constant_length() or self.is_identifier_length():
+return
+raise NotImplementedError('unknown array %s' % str(self))
+
def generate_c_declaration(self, writer, member):
+self.check_valid(member)
name = member.name
if member.has_attr("chunk"):
return writer.writeln('SpiceChunks *%s;' % name)
@@ -497,7 +513,7 @@ class ArrayType(Type):
return writer.writeln('%s *%s;' % (self.c_type(), name))
if member.has_attr("ptr_array"):
return writer.writeln('%s *%s[0];' % (self.c_type(), name))
-if member.has_end_attr() or self.is_remaining_length():
+if member.has_end_attr():
return writer.writeln('%s %s[0];' % (self.c_type(), name))
if self.is_constant_length():
return writer.writeln('%s %s[%s];' % (self.c_type(), name,
self.size))
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-common 3/4] codegen: Exit with error on error generating C structures
This was some left-over during development of C structure generations (the single structure generation was skipped). Signed-off-by: Frediano Ziglio --- spice_codegen.py | 1 + 1 file changed, 1 insertion(+) diff --git a/spice_codegen.py b/spice_codegen.py index 0532d6f..d3a1bf5 100755 --- a/spice_codegen.py +++ b/spice_codegen.py @@ -283,6 +283,7 @@ current: print >> sys.stderr, 'type %s' % t print >> sys.stderr, writer.getvalue() traceback.print_exc(sys.stderr) +sys.exit(1) def generate_declarations(): writer = codegen.CodeWriter() -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-common 2/4] codegen: Add a check for C structure fields
This check make sure that output fields for member with @end (arrays)
are declared as empty arrays in output C structure.
This avoids output fields to be declared as pointer or other
invalid types.
The check is a compile time check so no code in object file
is generated.
Signed-off-by: Frediano Ziglio
---
python_modules/demarshal.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index 6d8dbdd..acd4b6f 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -805,6 +805,9 @@ def write_array_parser(writer, member, nelements, array,
dest, scope):
if member:
array_start = dest.get_ref(member.name)
at_end = member.has_end_attr()
+# the field is supposed to be a [0] array, check it
+if at_end:
+writer.statement('verify(sizeof(%s) == 0)' % array_start)
else:
array_start = "end"
at_end = True
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-common 1/4] codegen: Use has_end_attr instead of has_attr("end")
Just style, they do the same thing, but is more coherent
with the rest of the code.
Signed-off-by: Frediano Ziglio
---
python_modules/demarshal.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index d3147b7..6d8dbdd 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -804,7 +804,7 @@ def write_array_parser(writer, member, nelements, array,
dest, scope):
element_type = array.element_type
if member:
array_start = dest.get_ref(member.name)
-at_end = member.has_attr("end")
+at_end = member.has_end_attr()
else:
array_start = "end"
at_end = True
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-common PATCH] test-marshallers.proto: ArrayMessage: make space for name
Do it by adding @end tag.
Without it the allocated memory has no space for 'name'.
Also fix SpiceMsgMainArrayMessage tests/test-marshallers.h,
replacing int8_t* name with int8_t name[].
This makes name an "in-structure" array with no pre-defined size
instead of a pointer.
The size is defined by the message size.
Signed-off-by: Uri Lublin
---
Since v1:
- fix 'name' in tests/test-marshallers.h too
- more information in the commit log
---
tests/test-marshallers.h | 2 +-
tests/test-marshallers.proto | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/test-marshallers.h b/tests/test-marshallers.h
index 7686067..8ca736e 100644
--- a/tests/test-marshallers.h
+++ b/tests/test-marshallers.h
@@ -10,7 +10,7 @@ typedef struct {
} SpiceMsgMainShortDataSubMarshall;
typedef struct {
-int8_t *name;
+int8_t name[0];
} SpiceMsgMainArrayMessage;
typedef struct {
diff --git a/tests/test-marshallers.proto b/tests/test-marshallers.proto
index 34cc892..eabd487 100644
--- a/tests/test-marshallers.proto
+++ b/tests/test-marshallers.proto
@@ -6,7 +6,7 @@ channel TestChannel {
} ShortDataSubMarshall;
message {
- int8 name[];
+ int8 name[] @end;
} ArrayMessage;
message {
--
2.21.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [Intel-gfx] [PATCH v6 08/17] drm/ttm: use gem vma_node
On Mon, Aug 05, 2019 at 04:01:10PM +0200, Gerd Hoffmann wrote:
> Drop vma_node from ttm_buffer_object, use the gem struct
> (base.vma_node) instead.
>
> Signed-off-by: Gerd Hoffmann
> Reviewed-by: Christian König
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +-
> drivers/gpu/drm/qxl/qxl_object.h | 2 +-
> drivers/gpu/drm/radeon/radeon_object.h | 2 +-
> drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +-
> include/drm/ttm/ttm_bo_api.h | 4
> drivers/gpu/drm/drm_gem_vram_helper.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_display.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +-
> drivers/gpu/drm/ttm/ttm_bo.c | 8
> drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +-
> drivers/gpu/drm/ttm/ttm_bo_vm.c| 9 +
> drivers/gpu/drm/virtio/virtgpu_prime.c | 3 ---
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_surface.c| 4 ++--
> 14 files changed, 21 insertions(+), 27 deletions(-)
Hi Gerd,
I've been seeing a regression on Nouveau with recent linux-next releases
and git bisect points at this commit as the first bad one. If I revert
it (there's a tiny conflict with a patch that was merged subsequently),
things are back to normal.
I think the reason for this issue is that Nouveau doesn't use GEM
objects for all buffer objects, and even when it uses GEM objects, the
code will not initialize the GEM object until after the buffer objects
and the backing TTM objects have been created.
I tried to fix that by making sure drm_gem_object_init() gets called by
Nouveau before ttm_bo_init(), but the changes are fairly involved and I
was unable to get the GEM reference counting right. I can look into the
proper fix some more, but it might be worth reverting this patch for
now to get Nouveau working again.
Thierry
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 645a189d365c..113fb2feb437 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -191,7 +191,7 @@ static inline unsigned
> amdgpu_bo_gpu_page_alignment(struct amdgpu_bo *bo)
> */
> static inline u64 amdgpu_bo_mmap_offset(struct amdgpu_bo *bo)
> {
> - return drm_vma_node_offset_addr(>tbo.vma_node);
> + return drm_vma_node_offset_addr(>tbo.base.vma_node);
> }
>
> /**
> diff --git a/drivers/gpu/drm/qxl/qxl_object.h
> b/drivers/gpu/drm/qxl/qxl_object.h
> index b812d4ae9d0d..8ae54ba7857c 100644
> --- a/drivers/gpu/drm/qxl/qxl_object.h
> +++ b/drivers/gpu/drm/qxl/qxl_object.h
> @@ -60,7 +60,7 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
>
> static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
> {
> - return drm_vma_node_offset_addr(>tbo.vma_node);
> + return drm_vma_node_offset_addr(>tbo.base.vma_node);
> }
>
> static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type,
> diff --git a/drivers/gpu/drm/radeon/radeon_object.h
> b/drivers/gpu/drm/radeon/radeon_object.h
> index 9ffd8215d38a..e5554bf9140e 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.h
> +++ b/drivers/gpu/drm/radeon/radeon_object.h
> @@ -116,7 +116,7 @@ static inline unsigned
> radeon_bo_gpu_page_alignment(struct radeon_bo *bo)
> */
> static inline u64 radeon_bo_mmap_offset(struct radeon_bo *bo)
> {
> - return drm_vma_node_offset_addr(>tbo.vma_node);
> + return drm_vma_node_offset_addr(>tbo.base.vma_node);
> }
>
> extern int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index f4ecea6054ba..e28829661724 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -396,7 +396,7 @@ static inline void virtio_gpu_object_unref(struct
> virtio_gpu_object **bo)
>
> static inline u64 virtio_gpu_object_mmap_offset(struct virtio_gpu_object *bo)
> {
> - return drm_vma_node_offset_addr(>tbo.vma_node);
> + return drm_vma_node_offset_addr(>tbo.base.vma_node);
> }
>
> static inline int virtio_gpu_object_reserve(struct virtio_gpu_object *bo,
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index fa050f0328ab..7ffc50a3303d 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -152,7 +152,6 @@ struct ttm_tt;
> * @ddestroy: List head for the delayed destroy list.
> * @swap: List head for swap LRU list.
> * @moving: Fence set when BO is moving
> - * @vma_node: Address space manager node.
> * @offset: The current GPU offset, which can have different meanings
> * depending on the memory type. For SYSTEM type memory, it should be 0.
> * @cur_placement: Hint of current placement.
> @@ -219,9 +218,6 @@ struct ttm_buffer_object {
>*/
>
> struct dma_fence *moving;
> -
> - struct drm_vma_offset_node vma_node;
> -
> unsigned
Re: [Spice-devel] [spice-common PATCH 2/2] test-marshallers.proto: ArrayMessage: make space for name
On 8/11/19 1:02 PM, Frediano Ziglio wrote:
Do it by adding @end tag.
Without it 'name' is a non-allocated pointer.
Signed-off-by: Uri Lublin
---
Is there a better way to do it ?
Is not clear what you are trying to achieve with this patch.
The problem is currently name is defined as a pointer
but is not allocated.
The generated code tries to memcpy data into name, which is
wrong. I see now the patch is missing a part -- more below.
---
tests/test-marshallers.proto | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/test-marshallers.proto b/tests/test-marshallers.proto
index 34cc892..eabd487 100644
--- a/tests/test-marshallers.proto
+++ b/tests/test-marshallers.proto
@@ -6,7 +6,7 @@ channel TestChannel {
} ShortDataSubMarshall;
message {
- int8 name[];
+ int8 name[] @end;
} ArrayMessage;
message {
Wondering where this message is used, I cannot find much references
to it beside autogenerated code.
I think it is not used.
There's a declaration for the type:
typedef struct {
int8_t *name;
} SpiceMsgMainArrayMessage;
Yes, this is wrong too -- it needs to be int8_t name[0].
I tried to use the code to generate the structure definitions and
I got this instead
typedef struct SpiceMsgMainArrayMessage {
int8_t name[0];
} SpiceMsgMainArrayMessage;
That's correct. Maybe we should auto-generate this .h file.
The demarshaller seems to not allocate space for the field which
seems quite a problem (maybe this is detected by Coverity??).
It is indeed detected by covscan.
I remember I had a patch with some notes about possible
security concerned mix of attributes, maybe this was one
situation.
There are no occurrences of this mix in spice.proto, either
you have @end or @as_ptr not arrays with unspecified length.
And this patch is adding @end.
I'll send a V2 -- changing tests/test-marshallers.h too
Thanks,
Uri.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v3 5/9] usb-redir: extend USB backend to support emulated devices
On Tue, Aug 13, 2019 at 2:59 PM Frediano Ziglio wrote:
>
> >
> > Redirection of emulated devices requires special approach,
> > as usbredirhost can't be used for that (it works only with
> > libusb devices). For emulated devices we create instance of
> > usbredirparser that implements USB redirection protocol.
>
> Make sense. Actually usbredirhost internally uses a parser to
> parse protocol messages and redirect to the physical device.
> But the parse is internal and is used by usbredirhost so
> would be hard to reuse.
>
> > In order to work with the same set of protocol capabilities
> > that usbredirhost sets up with remote side, the parser shall:
> > - not send its own 'hello' to the server
> > - initialize the same capabilities that usbredirhost
> > - receive the same 'hello' response
> > For that we analyze 'hello' sent by USB redir parser and
> > extract set of capabilities from it and upon receive of
> > 'hello' response we provide it also to the parser.
>
> This make sense but some part of the code does not exactly that.
> In some path the code send the 'hello' is sent by the parser.
> It would be more clear to implement just what you described
> above, why the implementation is a bit different.
Due to corner case described below.
>
> > When libusb device is redirected via a channel, instance of
> > usbredirhost serves it.
> > When emulated device is redirected via a channel, instance
> > of usbredirparser does the job.
> >
> > Signed-off-by: Yuri Benditovich
> > ---
> > src/usb-backend.c | 616 +++---
> > 1 file changed, 580 insertions(+), 36 deletions(-)
> >
> > diff --git a/src/usb-backend.c b/src/usb-backend.c
> > index aa11c79..9aee6c7 100644
> > --- a/src/usb-backend.c
> > +++ b/src/usb-backend.c
> > @@ -55,6 +55,7 @@ struct _SpiceUsbBackendDevice
> > gint ref_count;
> > SpiceUsbBackendChannel *attached_to;
> > UsbDeviceInformation device_info;
> > +gboolean edev_configured;
>
> This variable is never reset, I would expect that at least
> when device is detached (this should be equivalent to physical
> removal).
Probably, need to reset it on attach.
In fact, nobody uses it, the driver usually just sets the configuration.
> Why not moving to SpiceUsbBackendChannel?
>
Because it is hardly related to the channel.
> > };
> >
> > struct _SpiceUsbBackend
> > @@ -80,10 +81,20 @@ struct _SpiceUsbBackend
> > struct _SpiceUsbBackendChannel
> > {
> > struct usbredirhost *usbredirhost;
> > +struct usbredirhost *hidden_host;
> > +struct usbredirparser *parser;
> > +struct usbredirparser *hidden_parser;
>
> I don't understand all these variable.
> Won't be easier to have parser initialized for emulated
> devices and being NULL (so freed) when emulated device
> is detached?
When the channel is created it does not know what it will redirect.
The same channel is used for regular devices and emulated one. Both
parser and usbredirhost shall be internally synchronized with the
remote side.
If I'd create the parser each time the emulated device is attached I'd
need each time to initialize it with received hello from the second
side. We'd need to write additional code to process (even imaginable)
error cases (what if we can't create the parser etc).
Finally this appears to be more complicated than have everythign ready
from the beginning.
There is also corner case when the device is attached before the
channel is ready (redirect-on-connect setting). We decide on
usbredihost' or 'parser' depending on kind of attached device to
redirect.
If the channel is still not ready, the parser/usbredirhost can't send
anything till the channel is up. In this case switching back and forth
between usbredirhost becomes more complicated than current solution.
>
> > uint8_t *read_buf;
> > int read_buf_size;
> > struct usbredirfilter_rule *rules;
> > int rules_count;
> > +uint32_t host_caps;
> > +uint32_t parser_init_done : 1;
> > +uint32_t parser_with_hello : 1;
> > +uint32_t hello_done_parser : 1;
> > +uint32_t hello_sent: 1;
> > +uint32_t rejected : 1;
> > +uint32_t wait_disc_ack : 1;
>
> The "disc" naming is confusing, also taking into account the
> "Compact Disc" addition. Reading the code this "disc" is a shorten
> for "disconnect", maybe would be better to just use "wait_disconnect_ack"?
>
ok
> > SpiceUsbBackendDevice *attached;
> > SpiceUsbredirChannel *user_data;
>
> OT: this definitively should be renamed!
Probably in separate commit.
>
> > SpiceUsbBackend *backend;
> > @@ -355,7 +366,11 @@ gboolean
> > spice_usb_backend_device_isoch(SpiceUsbBackendDevice *dev)
> > gint i, j, k;
> > int rc;
> >
> > -g_return_val_if_fail(libdev != NULL, 0);
> > +g_return_val_if_fail(libdev != NULL || dev->edev != NULL, 0);
> > +if (dev->edev) {
> > +/* currently we do not emulate isoch devices */
> > +return FALSE;
> > +
Re: [Spice-devel] [PATCH spice-html5] Review the webm audio track header and remove the fixmes.
On 8/11/19 4:32 AM, Frediano Ziglio wrote:
Hi,
why this was not merged ?
I completely missed the ack at the time.
I do think the patch is fundamentally correct and an improvement. This
one arguably deserves a reprise of the audit I performed at the time to
ensure that the results are still correct. I was planning to do that in
September (and resubmit the uid patch at the same time).
For the record, I have an internal todo list of patches that were missed
to circle back to. You've now caught most of them, thank you
(websockets being the big one). I believe I still have one or two
patches to argument parsing on libcacard, and I do not have a patch, but
a concern, on the removal of the spice-controller. I also have a large
separate patch set against the kernel providing usbredir support.
Cheers,
Jeremy
Frediano
Ack
Thanks,
Pavel
On Fri, 2016-12-23 at 10:53 -0600, Jeremy White wrote:
This involved a review of the Firefox parsing code along
with the official specifcation, and setting these fields
to the specified default values.
Most notably, I have found that recent browsers do not need the
8 ms pre skip, and it seems to remove some audio lag to
switch to 0.
Signed-off-by: Jeremy White
---
webm.js | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/webm.js b/webm.js
index 789da14..72c1853 100644
--- a/webm.js
+++ b/webm.js
@@ -393,28 +393,39 @@ webm_SeekHead.prototype =
function webm_AudioTrackEntry()
{
+/*
+** In general, we follow this specification:
+** https://www.matroska.org/technical/specs/index.html
+** we supply the mandatory values, and a comment notes
+** where we differ from the default
+** There is one RECOMMENDED guideline we are omitting;
+** the OPUS codec_delay is recommended to be 80ms.
+** However, the spice server does not currently provide time
+** stamps that are offset by 80ms, so you effectively get an
+** 80ms lag with this setting.
+*/
this.id = WEBM_TRACK_ENTRY;
this.number = 1;
this.uid = 1;
this.type = 2; // Audio
this.flag_enabled = 1;
this.flag_default = 1;
-this.flag_forced = 1;
-this.flag_lacing = 0;
-this.min_cache = 0; // fixme - check
+this.flag_forced = 1; // Different than default; we wish to
force
+this.flag_lacing = 1;
+this.min_cache = 0;
this.max_block_addition_id = 0;
-this.codec_decode_all = 0; // fixme - check
-this.seek_pre_roll = 0; // 8000; // fixme - check
-this.codec_delay = 8000; // Must match
codec_private.preskip
+this.seek_pre_roll = 0;
+this.codec_delay = 0; // Must match codec_private.preskip
this.codec_id = "A_OPUS";
+this.codec_decode_all = 1;
this.audio = new webm_Audio(OPUS_FREQUENCY);
// See: http://tools.ietf.org/html/draft-terriberry-oggopus-01
this.codec_private = [ 0x4f, 0x70, 0x75, 0x73, 0x48, 0x65,
0x61, 0x64, // OpusHead
0x01, // Version
OPUS_CHANNELS,
- 0x00, 0x0F, // Preskip - 3840 samples -
should be 8ms at 48kHz
- 0x80, 0xbb, 0x00, 0x00, // 48000
+ 0x00, 0x00, // Preskip - 3840 samples
would be 8ms at 48kHz
+ 0x80, 0xbb, 0x00, 0x00, // nominal rate
- 48000
0x00, 0x00, // Output gain
0x00 // Channel mapping family
];
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-gtk v2] build: Replace "join_paths" with "/" operator
Supported by Meson 0.49 (required by Spice-GTK).
New syntax is shorter and is recommended in
https://mesonbuild.com/Release-notes-for-0-49-0.html
("Joining paths with /").
Signed-off-by: Frediano Ziglio
---
doc/reference/meson.build | 18 +-
man/meson.build | 2 +-
meson.build | 8
src/meson.build | 10 +-
vapi/meson.build | 2 +-
5 files changed, 20 insertions(+), 20 deletions(-)
Changes since v1:
- extend commit message
diff --git a/doc/reference/meson.build b/doc/reference/meson.build
index e0005f06..61c410f6 100644
--- a/doc/reference/meson.build
+++ b/doc/reference/meson.build
@@ -39,10 +39,10 @@ ignore_headers = [
spice_gtk_doc_dep = declare_dependency(link_with : [spice_client_gtk_lib,
spice_client_glib_lib])
glib_prefix = dependency('glib-2.0').get_pkgconfig_variable('prefix')
-glib_docpath = join_paths(glib_prefix, 'share', 'gtk-doc', 'html')
+glib_docpath = glib_prefix / 'share' / 'gtk-doc' / 'html'
gtk_prefix = dependency('gtk+-3.0').get_pkgconfig_variable('prefix')
-gtk_docpath = join_paths(gtk_prefix, 'share', 'gtk-doc', 'html')
-docpath = join_paths(spice_gtk_datadir, 'gtk-doc', 'html')
+gtk_docpath = gtk_prefix / 'share' / 'gtk-doc' / 'html'
+docpath = spice_gtk_datadir / 'gtk-doc' / 'html'
gnome.gtkdoc(meson.project_name(),
dependencies : spice_gtk_doc_dep,
@@ -55,14 +55,14 @@ gnome.gtkdoc(meson.project_name(),
'--deprecated-guards="SPICE_DISABLE_DEPRECATED"',
'--ignore-decorators="G_GNUC_INTERNAL"',
'--rebuild-types',
- join_paths(meson.build_root(), 'src', 'spice-version.h')
+ meson.build_root() / 'src' / 'spice-version.h'
],
- src_dir : join_paths(meson.source_root(), 'src'),
+ src_dir : meson.source_root() / 'src',
fixxref_args: [
'--html-dir=@0@'.format(docpath),
- '--extra-dir=@0@'.format(join_paths(glib_docpath, 'glib')),
- '--extra-dir=@0@'.format(join_paths(glib_docpath, 'gobject')),
- '--extra-dir=@0@'.format(join_paths(glib_docpath, 'gio')),
- '--extra-dir=@0@'.format(join_paths(gtk_docpath, 'gtk3')),
+ '--extra-dir=@0@'.format(glib_docpath / 'glib'),
+ '--extra-dir=@0@'.format(glib_docpath / 'gobject'),
+ '--extra-dir=@0@'.format(glib_docpath / 'gio'),
+ '--extra-dir=@0@'.format(gtk_docpath / 'gtk3'),
],
)
diff --git a/man/meson.build b/man/meson.build
index de07ba44..86c63666 100644
--- a/man/meson.build
+++ b/man/meson.build
@@ -5,7 +5,7 @@ if pod2man.found()
output : 'spice-client.1',
input : 'spice-client.pod',
install : true,
-install_dir : join_paths(spice_gtk_datadir, 'man', 'man1'),
+install_dir : spice_gtk_datadir / 'man' / 'man1',
build_by_default : true,
command : [pod2man, '-c', 'Spice-GTK Documentation', '@INPUT@',
'@OUTPUT@'])
endif
diff --git a/meson.build b/meson.build
index 7817c732..07dbb701 100644
--- a/meson.build
+++ b/meson.build
@@ -12,10 +12,10 @@ meson.add_dist_script('build-aux/meson-dist',
meson.project_version(), meson.sou
# global C defines
#
spice_gtk_prefix = get_option('prefix')
-spice_gtk_bindir = join_paths(spice_gtk_prefix, get_option('bindir'))
-spice_gtk_datadir = join_paths(spice_gtk_prefix, get_option('datadir'))
-spice_gtk_localedir = join_paths(spice_gtk_datadir, 'locale')
-spice_gtk_includedir = join_paths(spice_gtk_prefix, get_option('includedir'))
+spice_gtk_bindir = spice_gtk_prefix / get_option('bindir')
+spice_gtk_datadir = spice_gtk_prefix / get_option('datadir')
+spice_gtk_localedir = spice_gtk_datadir / 'locale'
+spice_gtk_includedir = spice_gtk_prefix / get_option('includedir')
spice_gtk_global_cflags = ['-DHAVE_CONFIG_H',
'-DSPICE_COMPILATION',
'-DG_LOG_DOMAIN="GSpice"',
diff --git a/src/meson.build b/src/meson.build
index be3f56c0..ede41c62 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -56,7 +56,7 @@ spice_marshals = gnome.genmarshal('spice-marshal', sources :
'spice-marshal.txt'
spice_client_glib_enums = gnome.mkenums_simple('spice-glib-enums',
sources : ['spice-channel.h',
'channel-inputs.h', 'spice-session.h'],
install_header : true,
- install_dir :
join_paths(spice_gtk_includedir, 'spice-client-glib-2.0'))
+ install_dir :
spice_gtk_includedir / 'spice-client-glib-2.0')
spice_client_glib_introspection_sources = [
spice_client_glib_headers,
@@ -175,12 +175,12 @@ endif
# version-script
spice_client_glib_syms = files('map-file')
-spice_client_glib_syms_path =
Re: [Spice-devel] [spice-gtk v3 5/9] usb-redir: extend USB backend to support emulated devices
>
> Redirection of emulated devices requires special approach,
> as usbredirhost can't be used for that (it works only with
> libusb devices). For emulated devices we create instance of
> usbredirparser that implements USB redirection protocol.
Make sense. Actually usbredirhost internally uses a parser to
parse protocol messages and redirect to the physical device.
But the parse is internal and is used by usbredirhost so
would be hard to reuse.
> In order to work with the same set of protocol capabilities
> that usbredirhost sets up with remote side, the parser shall:
> - not send its own 'hello' to the server
> - initialize the same capabilities that usbredirhost
> - receive the same 'hello' response
> For that we analyze 'hello' sent by USB redir parser and
> extract set of capabilities from it and upon receive of
> 'hello' response we provide it also to the parser.
This make sense but some part of the code does not exactly that.
In some path the code send the 'hello' is sent by the parser.
It would be more clear to implement just what you described
above, why the implementation is a bit different.
> When libusb device is redirected via a channel, instance of
> usbredirhost serves it.
> When emulated device is redirected via a channel, instance
> of usbredirparser does the job.
>
> Signed-off-by: Yuri Benditovich
> ---
> src/usb-backend.c | 616 +++---
> 1 file changed, 580 insertions(+), 36 deletions(-)
>
> diff --git a/src/usb-backend.c b/src/usb-backend.c
> index aa11c79..9aee6c7 100644
> --- a/src/usb-backend.c
> +++ b/src/usb-backend.c
> @@ -55,6 +55,7 @@ struct _SpiceUsbBackendDevice
> gint ref_count;
> SpiceUsbBackendChannel *attached_to;
> UsbDeviceInformation device_info;
> +gboolean edev_configured;
This variable is never reset, I would expect that at least
when device is detached (this should be equivalent to physical
removal).
Why not moving to SpiceUsbBackendChannel?
> };
>
> struct _SpiceUsbBackend
> @@ -80,10 +81,20 @@ struct _SpiceUsbBackend
> struct _SpiceUsbBackendChannel
> {
> struct usbredirhost *usbredirhost;
> +struct usbredirhost *hidden_host;
> +struct usbredirparser *parser;
> +struct usbredirparser *hidden_parser;
I don't understand all these variable.
Won't be easier to have parser initialized for emulated
devices and being NULL (so freed) when emulated device
is detached?
> uint8_t *read_buf;
> int read_buf_size;
> struct usbredirfilter_rule *rules;
> int rules_count;
> +uint32_t host_caps;
> +uint32_t parser_init_done : 1;
> +uint32_t parser_with_hello : 1;
> +uint32_t hello_done_parser : 1;
> +uint32_t hello_sent: 1;
> +uint32_t rejected : 1;
> +uint32_t wait_disc_ack : 1;
The "disc" naming is confusing, also taking into account the
"Compact Disc" addition. Reading the code this "disc" is a shorten
for "disconnect", maybe would be better to just use "wait_disconnect_ack"?
> SpiceUsbBackendDevice *attached;
> SpiceUsbredirChannel *user_data;
OT: this definitively should be renamed!
> SpiceUsbBackend *backend;
> @@ -355,7 +366,11 @@ gboolean
> spice_usb_backend_device_isoch(SpiceUsbBackendDevice *dev)
> gint i, j, k;
> int rc;
>
> -g_return_val_if_fail(libdev != NULL, 0);
> +g_return_val_if_fail(libdev != NULL || dev->edev != NULL, 0);
> +if (dev->edev) {
> +/* currently we do not emulate isoch devices */
> +return FALSE;
> +}
>
> rc = libusb_get_active_config_descriptor(libdev, _desc);
> if (rc) {
> @@ -390,13 +405,16 @@ from both the main thread as well as from the usb event
> handling thread */
> static void usbredir_write_flush_callback(void *user_data)
> {
> SpiceUsbBackendChannel *ch = user_data;
> -if (!ch->usbredirhost) {
> -/* just to be on the safe side */
> -return;
> -}
These check should be in a preparatory patch. Nothing wrong with this patch,
but the code (master) is confusing. usbredirhost is initialized as soon as
possible, can be NULL only before usbredirhost_open_full returns.
Actually can be NULL in this call for this reason (this callback is called
inside usbredirhost_open_full).
> if (is_channel_ready(ch->user_data)) {
> -SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch);
> -usbredirhost_write_guest_data(ch->usbredirhost);
> +if (ch->usbredirhost) {
> +SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch);
> +usbredirhost_write_guest_data(ch->usbredirhost);
> +} else if (ch->parser) {
> +SPICE_DEBUG("%s ch %p -> parser", __FUNCTION__, ch);
> +usbredirparser_do_write(ch->parser);
> +} else {
> +SPICE_DEBUG("%s ch %p (NOT ACTIVE)", __FUNCTION__, ch);
This third case should never be possible. This is one aspect that master
code is confusing about.
> +}
> }
Re: [Spice-devel] [PATCH spice-gtk 2/2] build: Replace "join_paths" with "/" operator
On 7/31/19 10:27 AM, Frediano Ziglio wrote:
Supported by Meson 0.49.
New syntax is shorter.
as recommended, in
https://mesonbuild.com/Release-notes-for-0-49-0.html
("Joining paths with /"),
But, is it too new ?
I see for example that RHEL-7 has Meson 0.45.
Oh, but Meson 0.49 is already required.
Uri.
Signed-off-by: Frediano Ziglio > ---
doc/reference/meson.build | 18 +-
man/meson.build | 2 +-
meson.build | 8
src/meson.build | 10 +-
vapi/meson.build | 2 +-
5 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/doc/reference/meson.build b/doc/reference/meson.build
index e0005f06..61c410f6 100644
--- a/doc/reference/meson.build
+++ b/doc/reference/meson.build
@@ -39,10 +39,10 @@ ignore_headers = [
spice_gtk_doc_dep = declare_dependency(link_with : [spice_client_gtk_lib,
spice_client_glib_lib])
glib_prefix = dependency('glib-2.0').get_pkgconfig_variable('prefix')
-glib_docpath = join_paths(glib_prefix, 'share', 'gtk-doc', 'html')
+glib_docpath = glib_prefix / 'share' / 'gtk-doc' / 'html'
gtk_prefix = dependency('gtk+-3.0').get_pkgconfig_variable('prefix')
-gtk_docpath = join_paths(gtk_prefix, 'share', 'gtk-doc', 'html')
-docpath = join_paths(spice_gtk_datadir, 'gtk-doc', 'html')
+gtk_docpath = gtk_prefix / 'share' / 'gtk-doc' / 'html'
+docpath = spice_gtk_datadir / 'gtk-doc' / 'html'
gnome.gtkdoc(meson.project_name(),
dependencies : spice_gtk_doc_dep,
@@ -55,14 +55,14 @@ gnome.gtkdoc(meson.project_name(),
'--deprecated-guards="SPICE_DISABLE_DEPRECATED"',
'--ignore-decorators="G_GNUC_INTERNAL"',
'--rebuild-types',
- join_paths(meson.build_root(), 'src', 'spice-version.h')
+ meson.build_root() / 'src' / 'spice-version.h'
],
- src_dir : join_paths(meson.source_root(), 'src'),
+ src_dir : meson.source_root() / 'src',
fixxref_args: [
'--html-dir=@0@'.format(docpath),
- '--extra-dir=@0@'.format(join_paths(glib_docpath, 'glib')),
- '--extra-dir=@0@'.format(join_paths(glib_docpath, 'gobject')),
- '--extra-dir=@0@'.format(join_paths(glib_docpath, 'gio')),
- '--extra-dir=@0@'.format(join_paths(gtk_docpath, 'gtk3')),
+ '--extra-dir=@0@'.format(glib_docpath / 'glib'),
+ '--extra-dir=@0@'.format(glib_docpath / 'gobject'),
+ '--extra-dir=@0@'.format(glib_docpath / 'gio'),
+ '--extra-dir=@0@'.format(gtk_docpath / 'gtk3'),
],
)
diff --git a/man/meson.build b/man/meson.build
index de07ba44..86c63666 100644
--- a/man/meson.build
+++ b/man/meson.build
@@ -5,7 +5,7 @@ if pod2man.found()
output : 'spice-client.1',
input : 'spice-client.pod',
install : true,
-install_dir : join_paths(spice_gtk_datadir, 'man', 'man1'),
+install_dir : spice_gtk_datadir / 'man' / 'man1',
build_by_default : true,
command : [pod2man, '-c', 'Spice-GTK Documentation', '@INPUT@',
'@OUTPUT@'])
endif
diff --git a/meson.build b/meson.build
index 7817c732..07dbb701 100644
--- a/meson.build
+++ b/meson.build
@@ -12,10 +12,10 @@ meson.add_dist_script('build-aux/meson-dist',
meson.project_version(), meson.sou
# global C defines
#
spice_gtk_prefix = get_option('prefix')
-spice_gtk_bindir = join_paths(spice_gtk_prefix, get_option('bindir'))
-spice_gtk_datadir = join_paths(spice_gtk_prefix, get_option('datadir'))
-spice_gtk_localedir = join_paths(spice_gtk_datadir, 'locale')
-spice_gtk_includedir = join_paths(spice_gtk_prefix, get_option('includedir'))
+spice_gtk_bindir = spice_gtk_prefix / get_option('bindir')
+spice_gtk_datadir = spice_gtk_prefix / get_option('datadir')
+spice_gtk_localedir = spice_gtk_datadir / 'locale'
+spice_gtk_includedir = spice_gtk_prefix / get_option('includedir')
spice_gtk_global_cflags = ['-DHAVE_CONFIG_H',
'-DSPICE_COMPILATION',
'-DG_LOG_DOMAIN="GSpice"',
diff --git a/src/meson.build b/src/meson.build
index a0bdbc2a..c53b85fa 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -56,7 +56,7 @@ spice_marshals = gnome.genmarshal('spice-marshal', sources :
'spice-marshal.txt'
spice_client_glib_enums = gnome.mkenums_simple('spice-glib-enums',
sources : ['spice-channel.h',
'channel-inputs.h', 'spice-session.h'],
install_header : true,
- install_dir :
join_paths(spice_gtk_includedir, 'spice-client-glib-2.0'))
+ install_dir :
spice_gtk_includedir / 'spice-client-glib-2.0')
spice_client_glib_introspection_sources = [
spice_client_glib_headers,
@@ -174,12
Re: [Spice-devel] [spice-gtk v3 8/9] usb-redir: enable redirection of emulated CD drive
> > Add implementation of emulated device to build. > Now it is possible to create emulated CD devices. > > Signed-off-by: Yuri Benditovich Would be better to compile only if needed, like https://gitlab.freedesktop.org/fziglio/spice-gtk/commit/f74443390902335a0233bb32e1d94507e5b9f5f4 ? > --- > src/meson.build | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/src/meson.build b/src/meson.build > index fe19c16..3837a9d 100644 > --- a/src/meson.build > +++ b/src/meson.build > @@ -124,6 +124,13 @@ spice_client_glib_sources = [ >'usb-backend.c', >'usb-emulation.h', >'usb-backend.h', > + 'usb-device-cd.c', > + 'usb-device-cd.h', > + 'cd-scsi.c', > + 'cd-scsi.h', > + 'cd-scsi-dev-params.h', > + 'cd-usb-bulk-msd.c', > + 'cd-usb-bulk-msd.h', >'vmcstream.c', >'vmcstream.h', > ] Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 spice-streaming-agent] gst-plugin: Shorten template declarations
>
> The typeUPtr templates are very similar except for the unref-function.
> This patch is replacing the templates with a macro which also accepts
> an unref-function and is using this macro to define all typeUPtr types.
>
> Signed-off-by: Snir Sheriber
> ---
>
> -Update commit message
> -Conform with spice style
>
> ---
> src/gst-plugin.cpp | 64 +++---
> 1 file changed, 20 insertions(+), 44 deletions(-)
>
> diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
> index 4d17dbc..f6991f4 100644
> --- a/src/gst-plugin.cpp
> +++ b/src/gst-plugin.cpp
> @@ -38,43 +38,19 @@ struct GstreamerEncoderSettings
> std::vector> prop_pairs;
> };
>
> -template
> -struct GstObjectDeleter {
> -void operator()(T* p)
> -{
> -gst_object_unref(p);
> -}
> -};
> -
> -template
> -using GstObjectUPtr = std::unique_ptr>;
> -
> -struct GstCapsDeleter {
> -void operator()(GstCaps* p)
> -{
> -gst_caps_unref(p);
> -}
> -};
> -
> -using GstCapsUPtr = std::unique_ptr;
> -
> -struct GstSampleDeleter {
> -void operator()(GstSample* p)
> -{
> -gst_sample_unref(p);
> -}
> -};
> -
> -using GstSampleUPtr = std::unique_ptr;
> -
> -struct GstBufferDeleter {
> -void operator()(GstBuffer* p)
> -{
> -gst_buffer_unref(p);
> -}
> -};
> -
> -using GstBufferUPtr = std::unique_ptr;
> +#define DECLARE_UPTR(type, func) \
> +struct type##Deleter { \
> +void operator()(type* p) \
> +{\
> +func(p); \
> +}\
> +}; \
> +using type##UPtr = std::unique_ptr;
Shouldn't these last 2 lines indented too?
> +
> +DECLARE_UPTR(GstBuffer, gst_buffer_unref)
> +DECLARE_UPTR(GstCaps, gst_caps_unref)
> +DECLARE_UPTR(GstSample, gst_sample_unref)
> +DECLARE_UPTR(GstElement, gst_object_unref)
>
> class GstreamerFrameCapture final : public FrameCapture
> {
> @@ -96,7 +72,7 @@ private:
> #if XLIB_CAPTURE
> void xlib_capture();
> #endif
> -GstObjectUPtr pipeline, capture, sink;
> +GstElementUPtr pipeline, capture, sink;
> GstSampleUPtr sample;
> GstMapInfo map = {};
> uint32_t last_width = ~0u, last_height = ~0u;
> @@ -210,7 +186,7 @@ GstElement
> *GstreamerFrameCapture::get_encoder_plugin(const GstreamerEncoderSett
>
> // Utility to add an element to a GstBin
> // This checks return value and update reference correctly
> -void gst_bin_add(GstBin *bin, const GstObjectUPtr )
> +void gst_bin_add(GstBin *bin, const GstElementUPtr )
> {
> if (::gst_bin_add(bin, elem.get())) {
> // ::gst_bin_add take ownership using floating references but
> @@ -226,24 +202,24 @@ void GstreamerFrameCapture::pipeline_init(const
> GstreamerEncoderSettings
> {
> gboolean link;
>
> -GstObjectUPtr pipeline(gst_pipeline_new("pipeline"));
> +GstElementUPtr pipeline(gst_pipeline_new("pipeline"));
> if (!pipeline) {
> throw std::runtime_error("Gstreamer's pipeline element cannot be
> created");
> }
> -GstObjectUPtr capture(get_capture_plugin(settings));
> +GstElementUPtr capture(get_capture_plugin(settings));
> if (!capture) {
> throw std::runtime_error("Gstreamer's capture element cannot be
> created");
> }
> -GstObjectUPtr
> convert(gst_element_factory_make("autovideoconvert", "convert"));
> +GstElementUPtr convert(gst_element_factory_make("autovideoconvert",
> "convert"));
> if (!convert) {
> throw std::runtime_error("Gstreamer's 'autovideoconvert' element
> cannot be created");
> }
> GstCapsUPtr sink_caps;
> -GstObjectUPtr encoder(get_encoder_plugin(settings,
> sink_caps));
> +GstElementUPtr encoder(get_encoder_plugin(settings, sink_caps));
> if (!encoder) {
> throw std::runtime_error("Gstreamer's encoder element cannot be
> created");
> }
> -GstObjectUPtr sink(gst_element_factory_make("appsink",
> "sink"));
> +GstElementUPtr sink(gst_element_factory_make("appsink", "sink"));
> if (!sink) {
> throw std::runtime_error("Gstreamer's appsink element cannot be
> created");
> }
Otherwise,
Acked-by: Frediano Ziglio
Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-protocol] qxl_dev: Add comment to QXLQUICData->data field
Recently a bug using this structure was fixed.
The bug involved understand the usage of this field so add some
note on the field for future reference.
Signed-off-by: Frediano Ziglio
---
spice/qxl_dev.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/spice/qxl_dev.h b/spice/qxl_dev.h
index c844d7b..3f56aae 100644
--- a/spice/qxl_dev.h
+++ b/spice/qxl_dev.h
@@ -740,6 +740,9 @@ typedef struct SPICE_ATTR_PACKED QXLSurfaceId {
typedef struct SPICE_ATTR_PACKED QXLQUICData {
uint32_t data_size;
+/* This data for QUIC is a QXLDataChunk.
+ * This differs from QXLBitmap where it's a reference to bitmap data or
+ * a reference to QXLDataChunk */
uint8_t data[0];
} QXLQUICData, QXLLZRGBData, QXLJPEGData;
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
