[Spice-devel] [PATCH 2/4] channel-display-gst: Add "byte-stream" as the stream format for h264

[Victor Toso]

Hi,

On Fri, Sep 15, 2023 at 05:51:14PM +0100, Frediano Ziglio wrote:
> Il giorno ven 15 set 2023 alle ore 01:33 Vivek Kasireddy
> <vivek.kasireddy at intel.com> ha scritto:
> >
> > Adding the string "stream-format=byte-stream" to decoder capabilities
> > (dec_caps) for h264/h265 codecs stops Gstreamer from complaining
> > about missing stream format.
> >
> > Cc: Frediano Ziglio <freddy77 at gmail.com>
> > Cc: Dongwon Kim <dongwon.kim at intel.com>
> > Cc: Jin Chung Teng <jin.chung.teng at intel.com>
> > Cc: Hazwan Arif Mazlan <hazwan.arif.mazlan at intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> > ---
> >  src/channel-display-priv.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
> > index 66c9cff..1a7590a 100644
> > --- a/src/channel-display-priv.h
> > +++ b/src/channel-display-priv.h
> > @@ -177,7 +177,7 @@ static const struct {
> >       * (hardcoded in spice-server), let's add it here to avoid the warning.
> >       */
> >      { SPICE_DISPLAY_CAP_CODEC_H264, "h264",
> > -      "h264parse ! avdec_h264", "video/x-h264" },
> > +      "h264parse ! avdec_h264", "video/x-h264,stream-format=byte-stream" },
> >
> >      /* SPICE_VIDEO_CODEC_TYPE_VP9 */
> >      { SPICE_DISPLAY_CAP_CODEC_VP9, "vp9",
> > @@ -185,7 +185,7 @@ static const struct {
> >
> >      /* SPICE_DISPLAY_CAP_CODEC_H265 */
> >      { SPICE_DISPLAY_CAP_CODEC_H265, "h265",
> > -      "h265parse ! avdec_h265", "video/x-h265" },
> > +      "h265parse ! avdec_h265", "video/x-h265,stream-format=byte-stream" },
> >
> >  };
> >
> 
> Hi,
>    I agree with this change. Specifically I'm getting the warning too
> even if my system is pretty updated.
> 
> This setting was removed by commit
> https://gitlab.freedesktop.org/spice/spice-gtk/-/commit/c9129ed202b00f4a74ea7a55de19150194257e77
> but the explanation about it was removed is not really clear to me.

Yes, I've put the chat with tim and slomo because it was tricky.

The important point imho could be:

 | toso  slomo: right, my point was that I would like to list the
 | h264 decoders that are available and I was using `video/x-h264,
 | stream-format=3Dbyte-stream` as input for
 | gst_element_factory_list_filter() .. and avdec_h264 is the one
 | not showing because it needs the aligment

So, the problem I had at that time was exactly that using
stream-format would prevent loading avdec_h264 as viable option.

> Personally I think this commit should be applied in any case, it makes
> sense independently of the series. Vivek, do you agree?
> 
> Victor, do you agree to this change?

If you test this and it lists & works with avdec_h264, I think it
is fine too.

Cheers,
Victor
-------------- 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/20230915/38ee167d/attachment.sig>