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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx> 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@xxxxxxxxx>
> > Cc: Dongwon Kim <dongwon.kim@xxxxxxxxx>
> > Cc: Jin Chung Teng <jin.chung.teng@xxxxxxxxx>
> > Cc: Hazwan Arif Mazlan <hazwan.arif.mazlan@xxxxxxxxx>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>
> > ---
> >  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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]