[Spice-devel] [PATCH spice-gtk v3 3/6] display-gst: include capabilities for h264

[Victor Toso]

Hi,

On Tue, Jun 06, 2017 at 05:17:21PM +0200, Christophe Fergeau wrote:
> On Tue, May 16, 2017 at 04:48:15PM +0200, Victor Toso wrote:
> > From: Victor Toso <me at victortoso.com>
> > 
> > As the comment states, incomplete GstCaps for h264 could trigger
> > errors in h264parse element, such as:
> > 
> >   gst_h264_parse_set_caps: video/x-h264 caps without
> >   codec_data or stream-format
> > 
> > This would make h264parse to ignore the capabilities that were set.
> > 
> > As spice-server is encoding as byte-stream it should be fine to set
> > this value here too.
> > 
> > Any other errors to h264 format should either be reported to
> > GStreamer or fixed by improving the spice-protocol.
> > 
> > The follow up patch will identify elements in GstRegistry based on
> > GstCaps so this is a necessary change to have.
> > 
> > This is also a preparatory patch to use Playbin element to create the
> > pipeline. Without this, Playbin or typefind will fail to recognize the
> > stream as H264.
> > 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > Signed-off-by: Victor Toso <me at victortoso.com>
> > ---
> >  src/channel-display-gst.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > index 538e75a..73c2d25 100644
> > --- a/src/channel-display-gst.c
> > +++ b/src/channel-display-gst.c
> > @@ -68,10 +68,11 @@ static struct {
> >      { "vp8dec", "caps=video/x-vp8" },
> >  
> >      /* SPICE_VIDEO_CODEC_TYPE_H264
> > -     * h264 streams detection works fine and setting an incomplete cap
> > -     * causes errors. So let typefind do all the work.
> > +     * If setting video/x-h264, h264parse will complain if we don't have the
> 
> "When setting .." ?

Fixed

> 
> > +     * stream-format or codec_data information. As stream-format is byte-stream
> > +     * (hardcodec in spice-server), let add it to avoid the warning.
>
> "hardcoded", "let's add it here"

Fixed, thanks.

>
> Not really sure about the implications, meaning, ..., but why not,

Shouldn't be something that will break anything just a default
configuration. If configuration is wrong it would/should re-check the
headers (like it was doing originally) and have the right pipeline in
the end.

> Acked-by: Christophe Fergeau <cfergeau at redhat.com>

Thanks,

>
> >       */
> > -    { "h264parse ! avdec_h264", "" },
> > +    { "h264parse ! avdec_h264", "caps=video/x-h264,stream-format=byte-stream" },
> >  
> >      /* SPICE_VIDEO_CODEC_TYPE_VP9 */
> >      { "vp9dec", "caps=video/x-vp9" },
> > -- 
> > 2.13.0
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170627/675fc4f0/attachment.sig>