[FFmpeg-devel] [PATCH] avformat/flvdec: use avio operation instead of pb->buf_ptr use

[FFmpeg-devel] [PATCH] avformat/flvdec: use avio operation instead of pb->buf_ptr use

[Steven Liu]

fix segfaults:
READ of size 1 at 0x6100000003b7 thread T0
    #0 0x7f928d in flv_same_video_codec ffmpeg/libavformat/flvdec.c:317:29
    #1 0x7f928d in flv_read_packet ffmpeg/libavformat/flvdec.c:1177
    #2 0x6ff32f in ff_read_packet ffmpeg/libavformat/demux.c:575:15
    #3 0x70a2fd in read_frame_internal ffmpeg/libavformat/demux.c:1263:15
    #4 0x71d158 in avformat_find_stream_info ffmpeg/libavformat/demux.c:2634:15
    #5 0x4c821b in LLVMFuzzerTestOneInput ffmpeg/tools/target_dem_fuzzer.c:206:11

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavformat/flvdec.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index 3fe21622f7..003e4d7691 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -313,8 +313,9 @@ static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, int
         return 1;
 
     if (flv->exheader) {
-        uint8_t *codec_id_str = (uint8_t *)s->pb->buf_ptr;
-        uint32_t codec_id = codec_id_str[3] | codec_id_str[2] << 8 | codec_id_str[1] << 16 | codec_id_str[0] << 24;
+        int64_t pos = avio_tell(s->pb);
+        uint32_t codec_id = avio_rb32(s->pb);
+        avio_seek(s->pb, pos, SEEK_SET);
         switch(codec_id) {
             case MKBETAG('h', 'v', 'c', '1'):
                 return vpar->codec_id == AV_CODEC_ID_HEVC;
-- 
2.40.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/flvdec: use avio operation instead of pb->buf_ptr use

[Hendrik Leppkes]

On Thu, Jul 27, 2023 at 4:38 AM Steven Liu <lq@chinaffmpeg.org> wrote:
>
> fix segfaults:
> READ of size 1 at 0x6100000003b7 thread T0
>     #0 0x7f928d in flv_same_video_codec ffmpeg/libavformat/flvdec.c:317:29
>     #1 0x7f928d in flv_read_packet ffmpeg/libavformat/flvdec.c:1177
>     #2 0x6ff32f in ff_read_packet ffmpeg/libavformat/demux.c:575:15
>     #3 0x70a2fd in read_frame_internal ffmpeg/libavformat/demux.c:1263:15
>     #4 0x71d158 in avformat_find_stream_info ffmpeg/libavformat/demux.c:2634:15
>     #5 0x4c821b in LLVMFuzzerTestOneInput ffmpeg/tools/target_dem_fuzzer.c:206:11
>
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>  libavformat/flvdec.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index 3fe21622f7..003e4d7691 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -313,8 +313,9 @@ static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, int
>          return 1;
>
>      if (flv->exheader) {
> -        uint8_t *codec_id_str = (uint8_t *)s->pb->buf_ptr;
> -        uint32_t codec_id = codec_id_str[3] | codec_id_str[2] << 8 | codec_id_str[1] << 16 | codec_id_str[0] << 24;
> +        int64_t pos = avio_tell(s->pb);
> +        uint32_t codec_id = avio_rb32(s->pb);
> +        avio_seek(s->pb, pos, SEEK_SET);
>          switch(codec_id) {
>              case MKBETAG('h', 'v', 'c', '1'):
>                  return vpar->codec_id == AV_CODEC_ID_HEVC;

You don't need to store the position, just do a relative seek
backwards by 4 bytes.
I would also recommend to include a call to ffio_ensure_seekback so it
works with streams.

eg. something like this:
ffio_ensure_seekback(s->pb, 4);
avio_rb32(s->pb);
avio_seek(s->pb, -4, SEEK_CUR);

Add appropriate error checking, in particular for ffio_ensure_seekback, etc.

- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2] avformat/flvdec: use avio operation instead of pb->buf_ptr use

[Steven Liu]

check ensure seekback 4 bytes before read 4 bytes from pb,
and seek back 4 byte from current position after read 4 bytes.

fix segfaults:
READ of size 1 at 0x6100000003b7 thread T0
    #0 0x7f928d in flv_same_video_codec ffmpeg/libavformat/flvdec.c:317:29
    #1 0x7f928d in flv_read_packet ffmpeg/libavformat/flvdec.c:1177
    #2 0x6ff32f in ff_read_packet ffmpeg/libavformat/demux.c:575:15
    #3 0x70a2fd in read_frame_internal ffmpeg/libavformat/demux.c:1263:15
    #4 0x71d158 in avformat_find_stream_info ffmpeg/libavformat/demux.c:2634:15
    #5 0x4c821b in LLVMFuzzerTestOneInput ffmpeg/tools/target_dem_fuzzer.c:206:11

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavformat/flvdec.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index 3fe21622f7..38f34567dd 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -35,6 +35,7 @@
 #include "libavutil/intreadwrite.h"
 #include "libavutil/mathematics.h"
 #include "avformat.h"
+#include "avio_internal.h"
 #include "demux.h"
 #include "internal.h"
 #include "flv.h"
@@ -313,8 +314,14 @@ static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, int
         return 1;
 
     if (flv->exheader) {
-        uint8_t *codec_id_str = (uint8_t *)s->pb->buf_ptr;
-        uint32_t codec_id = codec_id_str[3] | codec_id_str[2] << 8 | codec_id_str[1] << 16 | codec_id_str[0] << 24;
+        uint32_t codec_id = 0;
+        int ret = ffio_ensure_seekback(s->pb, 4);
+        if (ret < 0) {
+            av_log(s, AV_LOG_WARNING, "Could not ensure seekback, because %s", av_err2str(ret));
+            return 0;
+        }
+        codec_id = avio_rb32(s->pb);
+        avio_seek(s->pb, -4, SEEK_CUR);
         switch(codec_id) {
             case MKBETAG('h', 'v', 'c', '1'):
                 return vpar->codec_id == AV_CODEC_ID_HEVC;
-- 
2.40.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/flvdec: use avio operation instead of pb->buf_ptr use

[Steven Liu]

Hendrik Leppkes <h.leppkes@gmail.com> 于2023年7月27日周四 14:47写道：
Hi Hendrik,

>
> On Thu, Jul 27, 2023 at 4:38 AM Steven Liu <lq@chinaffmpeg.org> wrote:
> >
> > fix segfaults:
> > READ of size 1 at 0x6100000003b7 thread T0
> >     #0 0x7f928d in flv_same_video_codec ffmpeg/libavformat/flvdec.c:317:29
> >     #1 0x7f928d in flv_read_packet ffmpeg/libavformat/flvdec.c:1177
> >     #2 0x6ff32f in ff_read_packet ffmpeg/libavformat/demux.c:575:15
> >     #3 0x70a2fd in read_frame_internal ffmpeg/libavformat/demux.c:1263:15
> >     #4 0x71d158 in avformat_find_stream_info ffmpeg/libavformat/demux.c:2634:15
> >     #5 0x4c821b in LLVMFuzzerTestOneInput ffmpeg/tools/target_dem_fuzzer.c:206:11
> >
> > Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> > ---
> >  libavformat/flvdec.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> > index 3fe21622f7..003e4d7691 100644
> > --- a/libavformat/flvdec.c
> > +++ b/libavformat/flvdec.c
> > @@ -313,8 +313,9 @@ static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, int
> >          return 1;
> >
> >      if (flv->exheader) {
> > -        uint8_t *codec_id_str = (uint8_t *)s->pb->buf_ptr;
> > -        uint32_t codec_id = codec_id_str[3] | codec_id_str[2] << 8 | codec_id_str[1] << 16 | codec_id_str[0] << 24;
> > +        int64_t pos = avio_tell(s->pb);
> > +        uint32_t codec_id = avio_rb32(s->pb);
> > +        avio_seek(s->pb, pos, SEEK_SET);
> >          switch(codec_id) {
> >              case MKBETAG('h', 'v', 'c', '1'):
> >                  return vpar->codec_id == AV_CODEC_ID_HEVC;
>
> You don't need to store the position, just do a relative seek
> backwards by 4 bytes.
> I would also recommend to include a call to ffio_ensure_seekback so it
> works with streams.

Thanks for your suggestions.
i have resubmit patch v2:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230727073142.64813-1-lq@chinaffmpeg.org/


>
> eg. something like this:
> ffio_ensure_seekback(s->pb, 4);
> avio_rb32(s->pb);
> avio_seek(s->pb, -4, SEEK_CUR);
>
> Add appropriate error checking, in particular for ffio_ensure_seekback, etc.
>
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Thanks
Steven
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] avformat/flvdec: use avio operation instead of pb->buf_ptr use

[Marton Balint]

On Thu, 27 Jul 2023, Steven Liu wrote:

> check ensure seekback 4 bytes before read 4 bytes from pb,
> and seek back 4 byte from current position after read 4 bytes.
>
> fix segfaults:
> READ of size 1 at 0x6100000003b7 thread T0
>    #0 0x7f928d in flv_same_video_codec ffmpeg/libavformat/flvdec.c:317:29
>    #1 0x7f928d in flv_read_packet ffmpeg/libavformat/flvdec.c:1177
>    #2 0x6ff32f in ff_read_packet ffmpeg/libavformat/demux.c:575:15
>    #3 0x70a2fd in read_frame_internal ffmpeg/libavformat/demux.c:1263:15
>    #4 0x71d158 in avformat_find_stream_info ffmpeg/libavformat/demux.c:2634:15
>    #5 0x4c821b in LLVMFuzzerTestOneInput ffmpeg/tools/target_dem_fuzzer.c:206:11
>
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
> libavformat/flvdec.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index 3fe21622f7..38f34567dd 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -35,6 +35,7 @@
> #include "libavutil/intreadwrite.h"
> #include "libavutil/mathematics.h"
> #include "avformat.h"
> +#include "avio_internal.h"
> #include "demux.h"
> #include "internal.h"
> #include "flv.h"
> @@ -313,8 +314,14 @@ static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, int
>         return 1;
>
>     if (flv->exheader) {
> -        uint8_t *codec_id_str = (uint8_t *)s->pb->buf_ptr;
> -        uint32_t codec_id = codec_id_str[3] | codec_id_str[2] << 8 | codec_id_str[1] << 16 | codec_id_str[0] << 24;
> +        uint32_t codec_id = 0;
> +        int ret = ffio_ensure_seekback(s->pb, 4);
> +        if (ret < 0) {
> +            av_log(s, AV_LOG_WARNING, "Could not ensure seekback, because %s", av_err2str(ret));
> +            return 0;
> +        }
> +        codec_id = avio_rb32(s->pb);
> +        avio_seek(s->pb, -4, SEEK_CUR);

Can't you rework the code to not do any IO here? It is super confusing 
that a function called "flv_same_video_codec" actually reads stuff instead 
of using only its parameters to check it.

IMHO the fourcc should be read where VideoTagHeader is read, not here. And 
the fourcc should be passed as parameter to flv_same_video_codec. Or maybe 
you can use a new variable called videocodecid, and set it to either 
fourcc or legacy videocodecid, and pass only that to flv_same_video_codec.

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3] avformat/flvdec: move read fourcc output before flv_same_video_codec

[Steven Liu]

read fourcc for video codec after VideoTagHeader read.
passed as parameter to flv_same_video_codec.

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavformat/flvdec.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index 3fe21622f7..e3cceba5d0 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -35,6 +35,7 @@
 #include "libavutil/intreadwrite.h"
 #include "libavutil/mathematics.h"
 #include "avformat.h"
+#include "avio_internal.h"
 #include "demux.h"
 #include "internal.h"
 #include "flv.h"
@@ -304,27 +305,22 @@ static void flv_set_audio_codec(AVFormatContext *s, AVStream *astream,
     }
 }
 
-static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, int flags)
+static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, uint32_t extent_codec_id, int flags)
 {
     int flv_codecid = flags & FLV_VIDEO_CODECID_MASK;
-    FLVContext *flv = s->priv_data;
 
     if (!vpar->codec_id && !vpar->codec_tag)
         return 1;
 
-    if (flv->exheader) {
-        uint8_t *codec_id_str = (uint8_t *)s->pb->buf_ptr;
-        uint32_t codec_id = codec_id_str[3] | codec_id_str[2] << 8 | codec_id_str[1] << 16 | codec_id_str[0] << 24;
-        switch(codec_id) {
-            case MKBETAG('h', 'v', 'c', '1'):
-                return vpar->codec_id == AV_CODEC_ID_HEVC;
-            case MKBETAG('a', 'v', '0', '1'):
-                return vpar->codec_id == AV_CODEC_ID_AV1;
-            case MKBETAG('v', 'p', '0', '9'):
-                return vpar->codec_id == AV_CODEC_ID_VP9;
-            default:
-                break;
-        }
+    switch(extent_codec_id) {
+        case MKBETAG('h', 'v', 'c', '1'):
+            return vpar->codec_id == AV_CODEC_ID_HEVC;
+        case MKBETAG('a', 'v', '0', '1'):
+            return vpar->codec_id == AV_CODEC_ID_AV1;
+        case MKBETAG('v', 'p', '0', '9'):
+            return vpar->codec_id == AV_CODEC_ID_VP9;
+        default:
+            break;
     }
 
     switch (flv_codecid) {
@@ -1065,6 +1061,7 @@ static int flv_read_packet(AVFormatContext *s, AVPacket *pkt)
     AVStream *st    = NULL;
     int last = -1;
     int orig_size;
+    uint32_t extent_codec_id = 0;
 
 retry:
     /* pkt size is repeated at end. skip it */
@@ -1117,6 +1114,15 @@ retry:
          * */
         flv->exheader = (flags >> 7) & 1;
         size--;
+        if (flv->exheader) {
+            int ensure_seekback_status = ffio_ensure_seekback(s->pb, 4);
+            if (ensure_seekback_status < 0) {
+                av_log(s, AV_LOG_WARNING, "Could not ensure seekback\n");
+                return ensure_seekback_status;
+            }
+            extent_codec_id = avio_rb32(s->pb);
+            avio_seek(s->pb, -4, SEEK_CUR);
+        }
         if ((flags & FLV_VIDEO_FRAMETYPE_MASK) == FLV_FRAME_VIDEO_INFO_CMD)
             goto skip;
     } else if (type == FLV_TAG_TYPE_META) {
@@ -1174,7 +1180,7 @@ skip:
                 break;
         } else if (stream_type == FLV_STREAM_TYPE_VIDEO) {
             if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
-                (s->video_codec_id || flv_same_video_codec(s, st->codecpar, flags)))
+                (s->video_codec_id || flv_same_video_codec(s, st->codecpar, extent_codec_id, flags)))
                 break;
         } else if (stream_type == FLV_STREAM_TYPE_SUBTITLE) {
             if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE)
-- 
2.40.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/flvdec: move read fourcc output before flv_same_video_codec

[Hendrik Leppkes]

On Sat, Jul 29, 2023 at 2:26 AM Steven Liu <lq@chinaffmpeg.org> wrote:
>
> read fourcc for video codec after VideoTagHeader read.
> passed as parameter to flv_same_video_codec.
>

Add the same parameter to flv_set_video_codec, and then you can remove
the entire backwards seek and just read it and adjust the size like
all other parameters.

- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/flvdec: move read fourcc output before flv_same_video_codec

[Marton Balint]

On Sat, 29 Jul 2023, Hendrik Leppkes wrote:

> On Sat, Jul 29, 2023 at 2:26 AM Steven Liu <lq@chinaffmpeg.org> wrote:
>>
>> read fourcc for video codec after VideoTagHeader read.
>> passed as parameter to flv_same_video_codec.
>>
>
> Add the same parameter to flv_set_video_codec, and then you can remove
> the entire backwards seek and just read it and adjust the size like
> all other parameters.

As far as I see flv_set_video_codec should not read a binary fourcc in the 
first place when it is called from amf_parse_object.

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/flvdec: move read fourcc output before flv_same_video_codec

[Hendrik Leppkes]

On Sat, Jul 29, 2023 at 10:46 AM Marton Balint <cus@passwd.hu> wrote:
>
>
>
> On Sat, 29 Jul 2023, Hendrik Leppkes wrote:
>
> > On Sat, Jul 29, 2023 at 2:26 AM Steven Liu <lq@chinaffmpeg.org> wrote:
> >>
> >> read fourcc for video codec after VideoTagHeader read.
> >> passed as parameter to flv_same_video_codec.
> >>
> >
> > Add the same parameter to flv_set_video_codec, and then you can remove
> > the entire backwards seek and just read it and adjust the size like
> > all other parameters.
>
> As far as I see flv_set_video_codec should not read a binary fourcc in the
> first place when it is called from amf_parse_object.

Yeah probably not, so moving it to a parameter would fix that as well,
and make the entire parsing flow less opaque.

- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/flvdec: move read fourcc output before flv_same_video_codec

[Hendrik Leppkes]

On Sat, Jul 29, 2023 at 11:54 AM Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>
> On Sat, Jul 29, 2023 at 10:46 AM Marton Balint <cus@passwd.hu> wrote:
> >
> >
> >
> > On Sat, 29 Jul 2023, Hendrik Leppkes wrote:
> >
> > > On Sat, Jul 29, 2023 at 2:26 AM Steven Liu <lq@chinaffmpeg.org> wrote:
> > >>
> > >> read fourcc for video codec after VideoTagHeader read.
> > >> passed as parameter to flv_same_video_codec.
> > >>
> > >
> > > Add the same parameter to flv_set_video_codec, and then you can remove
> > > the entire backwards seek and just read it and adjust the size like
> > > all other parameters.
> >
> > As far as I see flv_set_video_codec should not read a binary fourcc in the
> > first place when it is called from amf_parse_object.
>
> Yeah probably not, so moving it to a parameter would fix that as well,
> and make the entire parsing flow less opaque.
>

In fact the actual value will carry the FourCC then, which is not
handled at all. Definitely very broken here.

- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/flvdec: move read fourcc output before flv_same_video_codec

[Steven Liu]

Marton Balint <cus@passwd.hu>于2023年7月29日 周六16:45写道：

>
>
> On Sat, 29 Jul 2023, Hendrik Leppkes wrote:
>
> > On Sat, Jul 29, 2023 at 2:26 AM Steven Liu <lq@chinaffmpeg.org> wrote:
> >>
> >> read fourcc for video codec after VideoTagHeader read.
> >> passed as parameter to flv_same_video_codec.
> >>
> >
> > Add the same parameter to flv_set_video_codec, and then you can remove
> > the entire backwards seek and just read it and adjust the size like
> > all other parameters.
>
> As far as I see flv_set_video_codec should not read a binary fourcc in the
> first place when it is called from amf_parse_object.

yes i looked in flv_set_video_codec, it should not do the same operation,
because it cannot read codeid in exheader mode.

>
>
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avformat/flvdec: handle exheader fourcc correctly in metadata

[Marton Balint]

In metadata fourcc is carried in the AMF number, not as binary.

Partially based on a patch by Steven Liu.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/flvdec.c | 76 +++++++++++++++++---------------------------
 1 file changed, 30 insertions(+), 46 deletions(-)

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index 3fe21622f7..b904029ba5 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -304,30 +304,18 @@ static void flv_set_audio_codec(AVFormatContext *s, AVStream *astream,
     }
 }
 
-static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, int flags)
+static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, uint32_t flv_codecid)
 {
-    int flv_codecid = flags & FLV_VIDEO_CODECID_MASK;
-    FLVContext *flv = s->priv_data;
-
     if (!vpar->codec_id && !vpar->codec_tag)
         return 1;
 
-    if (flv->exheader) {
-        uint8_t *codec_id_str = (uint8_t *)s->pb->buf_ptr;
-        uint32_t codec_id = codec_id_str[3] | codec_id_str[2] << 8 | codec_id_str[1] << 16 | codec_id_str[0] << 24;
-        switch(codec_id) {
-            case MKBETAG('h', 'v', 'c', '1'):
-                return vpar->codec_id == AV_CODEC_ID_HEVC;
-            case MKBETAG('a', 'v', '0', '1'):
-                return vpar->codec_id == AV_CODEC_ID_AV1;
-            case MKBETAG('v', 'p', '0', '9'):
-                return vpar->codec_id == AV_CODEC_ID_VP9;
-            default:
-                break;
-        }
-    }
-
     switch (flv_codecid) {
+    case MKBETAG('h', 'v', 'c', '1'):
+        return vpar->codec_id == AV_CODEC_ID_HEVC;
+    case MKBETAG('a', 'v', '0', '1'):
+        return vpar->codec_id == AV_CODEC_ID_AV1;
+    case MKBETAG('v', 'p', '0', '9'):
+        return vpar->codec_id == AV_CODEC_ID_VP9;
     case FLV_CODECID_H263:
         return vpar->codec_id == AV_CODEC_ID_FLV1;
     case FLV_CODECID_SCREEN:
@@ -346,36 +334,26 @@ static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, int
 }
 
 static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream,
-                               int flv_codecid, int read)
+                               uint32_t flv_codecid, int read)
 {
     FFStream *const vstreami = ffstream(vstream);
-    FLVContext *flv = s->priv_data;
     int ret = 0;
     AVCodecParameters *par = vstream->codecpar;
     enum AVCodecID old_codec_id = vstream->codecpar->codec_id;
-    flv_codecid &= FLV_VIDEO_CODECID_MASK;
-
-    if (flv->exheader) {
-        uint32_t codec_id = avio_rb32(s->pb);
-
-        switch(codec_id) {
-            case MKBETAG('h', 'v', 'c', '1'):
-                par->codec_id = AV_CODEC_ID_HEVC;
-                vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
-                return 4;
-            case MKBETAG('a', 'v', '0', '1'):
-                par->codec_id = AV_CODEC_ID_AV1;
-                vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
-                return 4;
-            case MKBETAG('v', 'p', '0', '9'):
-                par->codec_id = AV_CODEC_ID_VP9;
-                vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
-                return 4;
-            default:
-                break;
-        }
-    }
-    switch (flv_codecid) {
+
+    switch(flv_codecid) {
+    case MKBETAG('h', 'v', 'c', '1'):
+        par->codec_id = AV_CODEC_ID_HEVC;
+        vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
+        break;
+    case MKBETAG('a', 'v', '0', '1'):
+        par->codec_id = AV_CODEC_ID_AV1;
+        vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
+        break;
+    case MKBETAG('v', 'p', '0', '9'):
+        par->codec_id = AV_CODEC_ID_VP9;
+        vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
+        break;
     case FLV_CODECID_H263:
         par->codec_id = AV_CODEC_ID_FLV1;
         break;
@@ -1065,6 +1043,7 @@ static int flv_read_packet(AVFormatContext *s, AVPacket *pkt)
     AVStream *st    = NULL;
     int last = -1;
     int orig_size;
+    uint32_t video_codec_id = 0;
 
 retry:
     /* pkt size is repeated at end. skip it */
@@ -1111,12 +1090,17 @@ retry:
     } else if (type == FLV_TAG_TYPE_VIDEO) {
         stream_type = FLV_STREAM_TYPE_VIDEO;
         flags    = avio_r8(s->pb);
+        video_codec_id = flags & FLV_VIDEO_CODECID_MASK;
         /*
          * Reference Enhancing FLV 2023-03-v1.0.0-B.8
          * https://github.com/veovera/enhanced-rtmp/blob/main/enhanced-rtmp-v1.pdf
          * */
         flv->exheader = (flags >> 7) & 1;
         size--;
+        if (flv->exheader) {
+            video_codec_id = avio_rb32(s->pb);
+            size -= 4;
+        }
         if ((flags & FLV_VIDEO_FRAMETYPE_MASK) == FLV_FRAME_VIDEO_INFO_CMD)
             goto skip;
     } else if (type == FLV_TAG_TYPE_META) {
@@ -1174,7 +1158,7 @@ skip:
                 break;
         } else if (stream_type == FLV_STREAM_TYPE_VIDEO) {
             if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
-                (s->video_codec_id || flv_same_video_codec(s, st->codecpar, flags)))
+                (s->video_codec_id || flv_same_video_codec(s, st->codecpar, video_codec_id)))
                 break;
         } else if (stream_type == FLV_STREAM_TYPE_SUBTITLE) {
             if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE)
@@ -1275,7 +1259,7 @@ retry_duration:
             avcodec_parameters_free(&par);
         }
     } else if (stream_type == FLV_STREAM_TYPE_VIDEO) {
-        int ret = flv_set_video_codec(s, st, flags, 1);
+        int ret = flv_set_video_codec(s, st, video_codec_id, 1);
         if (ret < 0)
             return ret;
         size -= ret;
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/flvdec: handle exheader fourcc correctly in metadata

[Steven Liu]

Marton Balint <cus@passwd.hu> 于2023年7月30日周日 01:49写道：
>
> In metadata fourcc is carried in the AMF number, not as binary.
>
> Partially based on a patch by Steven Liu.
>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/flvdec.c | 76 +++++++++++++++++---------------------------
>  1 file changed, 30 insertions(+), 46 deletions(-)
>
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index 3fe21622f7..b904029ba5 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -304,30 +304,18 @@ static void flv_set_audio_codec(AVFormatContext *s, AVStream *astream,
>      }
>  }
>
> -static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, int flags)
> +static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, uint32_t flv_codecid)
>  {
> -    int flv_codecid = flags & FLV_VIDEO_CODECID_MASK;
> -    FLVContext *flv = s->priv_data;
> -
>      if (!vpar->codec_id && !vpar->codec_tag)
>          return 1;
>
> -    if (flv->exheader) {
> -        uint8_t *codec_id_str = (uint8_t *)s->pb->buf_ptr;
> -        uint32_t codec_id = codec_id_str[3] | codec_id_str[2] << 8 | codec_id_str[1] << 16 | codec_id_str[0] << 24;
> -        switch(codec_id) {
> -            case MKBETAG('h', 'v', 'c', '1'):
> -                return vpar->codec_id == AV_CODEC_ID_HEVC;
> -            case MKBETAG('a', 'v', '0', '1'):
> -                return vpar->codec_id == AV_CODEC_ID_AV1;
> -            case MKBETAG('v', 'p', '0', '9'):
> -                return vpar->codec_id == AV_CODEC_ID_VP9;
> -            default:
> -                break;
> -        }
> -    }
> -
>      switch (flv_codecid) {
> +    case MKBETAG('h', 'v', 'c', '1'):
> +        return vpar->codec_id == AV_CODEC_ID_HEVC;
> +    case MKBETAG('a', 'v', '0', '1'):
> +        return vpar->codec_id == AV_CODEC_ID_AV1;
> +    case MKBETAG('v', 'p', '0', '9'):
> +        return vpar->codec_id == AV_CODEC_ID_VP9;
>      case FLV_CODECID_H263:
>          return vpar->codec_id == AV_CODEC_ID_FLV1;
>      case FLV_CODECID_SCREEN:
> @@ -346,36 +334,26 @@ static int flv_same_video_codec(AVFormatContext *s, AVCodecParameters *vpar, int
>  }
>
>  static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream,
> -                               int flv_codecid, int read)
> +                               uint32_t flv_codecid, int read)
>  {
>      FFStream *const vstreami = ffstream(vstream);
> -    FLVContext *flv = s->priv_data;
>      int ret = 0;
>      AVCodecParameters *par = vstream->codecpar;
>      enum AVCodecID old_codec_id = vstream->codecpar->codec_id;
> -    flv_codecid &= FLV_VIDEO_CODECID_MASK;
> -
> -    if (flv->exheader) {
> -        uint32_t codec_id = avio_rb32(s->pb);
> -
> -        switch(codec_id) {
> -            case MKBETAG('h', 'v', 'c', '1'):
> -                par->codec_id = AV_CODEC_ID_HEVC;
> -                vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
> -                return 4;
> -            case MKBETAG('a', 'v', '0', '1'):
> -                par->codec_id = AV_CODEC_ID_AV1;
> -                vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
> -                return 4;
> -            case MKBETAG('v', 'p', '0', '9'):
> -                par->codec_id = AV_CODEC_ID_VP9;
> -                vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
> -                return 4;
> -            default:
> -                break;
> -        }
> -    }
> -    switch (flv_codecid) {
> +
> +    switch(flv_codecid) {
> +    case MKBETAG('h', 'v', 'c', '1'):
> +        par->codec_id = AV_CODEC_ID_HEVC;
> +        vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
> +        break;
> +    case MKBETAG('a', 'v', '0', '1'):
> +        par->codec_id = AV_CODEC_ID_AV1;
> +        vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
> +        break;
> +    case MKBETAG('v', 'p', '0', '9'):
> +        par->codec_id = AV_CODEC_ID_VP9;
> +        vstreami->need_parsing = AVSTREAM_PARSE_HEADERS;
> +        break;
>      case FLV_CODECID_H263:
>          par->codec_id = AV_CODEC_ID_FLV1;
>          break;
> @@ -1065,6 +1043,7 @@ static int flv_read_packet(AVFormatContext *s, AVPacket *pkt)
>      AVStream *st    = NULL;
>      int last = -1;
>      int orig_size;
> +    uint32_t video_codec_id = 0;
>
>  retry:
>      /* pkt size is repeated at end. skip it */
> @@ -1111,12 +1090,17 @@ retry:
>      } else if (type == FLV_TAG_TYPE_VIDEO) {
>          stream_type = FLV_STREAM_TYPE_VIDEO;
>          flags    = avio_r8(s->pb);
> +        video_codec_id = flags & FLV_VIDEO_CODECID_MASK;
>          /*
>           * Reference Enhancing FLV 2023-03-v1.0.0-B.8
>           * https://github.com/veovera/enhanced-rtmp/blob/main/enhanced-rtmp-v1.pdf
>           * */
>          flv->exheader = (flags >> 7) & 1;
>          size--;
> +        if (flv->exheader) {
> +            video_codec_id = avio_rb32(s->pb);
> +            size -= 4;
> +        }
>          if ((flags & FLV_VIDEO_FRAMETYPE_MASK) == FLV_FRAME_VIDEO_INFO_CMD)
>              goto skip;
>      } else if (type == FLV_TAG_TYPE_META) {
> @@ -1174,7 +1158,7 @@ skip:
>                  break;
>          } else if (stream_type == FLV_STREAM_TYPE_VIDEO) {
>              if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> -                (s->video_codec_id || flv_same_video_codec(s, st->codecpar, flags)))
> +                (s->video_codec_id || flv_same_video_codec(s, st->codecpar, video_codec_id)))
>                  break;
>          } else if (stream_type == FLV_STREAM_TYPE_SUBTITLE) {
>              if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE)
> @@ -1275,7 +1259,7 @@ retry_duration:
>              avcodec_parameters_free(&par);
>          }
>      } else if (stream_type == FLV_STREAM_TYPE_VIDEO) {
> -        int ret = flv_set_video_codec(s, st, flags, 1);
> +        int ret = flv_set_video_codec(s, st, video_codec_id, 1);
>          if (ret < 0)
>              return ret;
>          size -= ret;
> --
> 2.35.3
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

should be ok

Thanks
Steven
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/flvdec: handle exheader fourcc correctly in metadata

[Marton Balint]

On Wed, 2 Aug 2023, Steven Liu wrote:

> Marton Balint <cus@passwd.hu> 于2023年7月30日周日 01:49写道：
>>
>> In metadata fourcc is carried in the AMF number, not as binary.
>>
>> Partially based on a patch by Steven Liu.
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/flvdec.c | 76 +++++++++++++++++---------------------------
>>  1 file changed, 30 insertions(+), 46 deletions(-)
>>

> should be ok

Will apply.

Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".