Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
@ 2025-04-24  3:38 jiangjie
  2025-04-24  4:19 ` Zhao Zhili
  2025-04-24 12:42 ` jiangjie
  0 siblings, 2 replies; 11+ messages in thread
From: jiangjie @ 2025-04-24  3:38 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: jiangjie618

if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter will return an error.
ffmpeg -i rtmp://xxx/live/xxx -bsf:v "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -
---
 libavcodec/bsf/h264_mp4toannexb.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/libavcodec/bsf/h264_mp4toannexb.c b/libavcodec/bsf/h264_mp4toannexb.c
index dda064287e..4cd002d166 100644
--- a/libavcodec/bsf/h264_mp4toannexb.c
+++ b/libavcodec/bsf/h264_mp4toannexb.c
@@ -252,14 +252,21 @@ static int h264_mp4toannexb_filter_ps(H264BSFContext *s,
     return 0;
 }
 
+static int is_annexb(const uint8_t *extradata, int extra_size) {
+    if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) ||
+        (extra_size >= 4 && AV_RB32(extradata) == 1)) {
+        return 1;
+    } else {
+        return 0;
+    }
+}
+
 static int h264_mp4toannexb_init(AVBSFContext *ctx)
 {
     int extra_size = ctx->par_in->extradata_size;
 
     /* retrieve sps and pps NAL units from extradata */
-    if (!extra_size                                               ||
-        (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) ||
-        (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
+    if (is_annexb(ctx->par_in->extradata, extra_size)) {
         av_log(ctx, AV_LOG_VERBOSE,
                "The input looks like it is Annex B already\n");
     } else if (extra_size >= 7) {
@@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt)
     extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
                                         &extradata_size);
     if (extradata) {
-        ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
-        if (ret < 0)
-            goto fail;
+        if (!is_annexb(extradata, extradata_size) && extradata_size >= 7) {
+            ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
+            if (ret < 0)
+                goto fail;
+            av_packet_side_data_remove(in->side_data, &in->side_data_elems,
+                                       AV_PKT_DATA_NEW_EXTRADATA);
+            av_packet_add_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
+                                    ctx->par_out->extradata,
+                                    ctx->par_out->extradata_size);
+        }
     }
 
     /* nothing to filter */
-- 
2.49.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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
  2025-04-24  3:38 [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc jiangjie
@ 2025-04-24  4:19 ` Zhao Zhili
  2025-04-24  5:18   ` jie jiang
  2025-04-24 12:42 ` jiangjie
  1 sibling, 1 reply; 11+ messages in thread
From: Zhao Zhili @ 2025-04-24  4:19 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: jiangjie618



> On Apr 24, 2025, at 11:38, jiangjie <jiangjie618@gmail.com> wrote:
> 
> if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter will return an error.
> ffmpeg -i rtmp://xxx/live/xxx -bsf:v "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -

H.264 in FLV is avcc. Which service provider use annexb in FLV?

And I don’t think it makes sense to use AV_PKT_DATA_NEW_EXTRADATA together with annexb.

> ---
> libavcodec/bsf/h264_mp4toannexb.c | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/bsf/h264_mp4toannexb.c b/libavcodec/bsf/h264_mp4toannexb.c
> index dda064287e..4cd002d166 100644
> --- a/libavcodec/bsf/h264_mp4toannexb.c
> +++ b/libavcodec/bsf/h264_mp4toannexb.c
> @@ -252,14 +252,21 @@ static int h264_mp4toannexb_filter_ps(H264BSFContext *s,
>     return 0;
> }
> 
> +static int is_annexb(const uint8_t *extradata, int extra_size) {
> +    if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) ||
> +        (extra_size >= 4 && AV_RB32(extradata) == 1)) {
> +        return 1;
> +    } else {
> +        return 0;
> +    }
> +}
> +
> static int h264_mp4toannexb_init(AVBSFContext *ctx)
> {
>     int extra_size = ctx->par_in->extradata_size;
> 
>     /* retrieve sps and pps NAL units from extradata */
> -    if (!extra_size                                               ||
> -        (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) ||
> -        (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
> +    if (is_annexb(ctx->par_in->extradata, extra_size)) {
>         av_log(ctx, AV_LOG_VERBOSE,
>                "The input looks like it is Annex B already\n");
>     } else if (extra_size >= 7) {
> @@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt)
>     extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
>                                         &extradata_size);
>     if (extradata) {
> -        ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
> -        if (ret < 0)
> -            goto fail;
> +        if (!is_annexb(extradata, extradata_size) && extradata_size >= 7) {
> +            ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
> +            if (ret < 0)
> +                goto fail;
> +            av_packet_side_data_remove(in->side_data, &in->side_data_elems,
> +                                       AV_PKT_DATA_NEW_EXTRADATA);
> +            av_packet_add_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
> +                                    ctx->par_out->extradata,
> +                                    ctx->par_out->extradata_size);
> +        }
>     }
> 
>     /* nothing to filter */
> -- 
> 2.49.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".

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
  2025-04-24  4:19 ` Zhao Zhili
@ 2025-04-24  5:18   ` jie jiang
  2025-04-24  6:41     ` Zhao Zhili
  0 siblings, 1 reply; 11+ messages in thread
From: jie jiang @ 2025-04-24  5:18 UTC (permalink / raw)
  To: Zhao Zhili; +Cc: FFmpeg development discussions and patches

fmpeg -i rtmp://xxx/live/xxx -bsf:v
"h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -

For h264_mp4toannexb, input 264 is avcc,  output is annexb. and for
h264_metadata, both input and output are annexb.
If the sps/pps header is updated,  flvdec module will insert
AV_PKT_DATA_NEW_EXTRADATA to AVPacket and is this format of header is avcc.
In cbs_bsf_update_side_data function, it will find
AV_PKT_DATA_NEW_EXTRADATA and this extradata is avcc, that will cause
h264_metadata to judge that its input is also avcc, but  its input  is from
h264_mp4toannexb. Using the logic of parsing avcc to parse annexb is
problematic.




On Thu, Apr 24, 2025 at 12:19 PM Zhao Zhili <quinkblack@foxmail.com> wrote:

>
>
> > On Apr 24, 2025, at 11:38, jiangjie <jiangjie618@gmail.com> wrote:
> >
> > if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter will
> return an error.
> > ffmpeg -i rtmp://xxx/live/xxx -bsf:v
> "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -
>
> H.264 in FLV is avcc. Which service provider use annexb in FLV?
>
> And I don’t think it makes sense to use AV_PKT_DATA_NEW_EXTRADATA together
> with annexb.
>
> > ---
> > libavcodec/bsf/h264_mp4toannexb.c | 26 ++++++++++++++++++++------
> > 1 file changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/bsf/h264_mp4toannexb.c
> b/libavcodec/bsf/h264_mp4toannexb.c
> > index dda064287e..4cd002d166 100644
> > --- a/libavcodec/bsf/h264_mp4toannexb.c
> > +++ b/libavcodec/bsf/h264_mp4toannexb.c
> > @@ -252,14 +252,21 @@ static int
> h264_mp4toannexb_filter_ps(H264BSFContext *s,
> >     return 0;
> > }
> >
> > +static int is_annexb(const uint8_t *extradata, int extra_size) {
> > +    if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) ||
> > +        (extra_size >= 4 && AV_RB32(extradata) == 1)) {
> > +        return 1;
> > +    } else {
> > +        return 0;
> > +    }
> > +}
> > +
> > static int h264_mp4toannexb_init(AVBSFContext *ctx)
> > {
> >     int extra_size = ctx->par_in->extradata_size;
> >
> >     /* retrieve sps and pps NAL units from extradata */
> > -    if (!extra_size                                               ||
> > -        (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) ||
> > -        (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
> > +    if (is_annexb(ctx->par_in->extradata, extra_size)) {
> >         av_log(ctx, AV_LOG_VERBOSE,
> >                "The input looks like it is Annex B already\n");
> >     } else if (extra_size >= 7) {
> > @@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext
> *ctx, AVPacket *opkt)
> >     extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
> >                                         &extradata_size);
> >     if (extradata) {
> > -        ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
> > -        if (ret < 0)
> > -            goto fail;
> > +        if (!is_annexb(extradata, extradata_size) && extradata_size >=
> 7) {
> > +            ret = h264_extradata_to_annexb(ctx, extradata,
> extradata_size);
> > +            if (ret < 0)
> > +                goto fail;
> > +            av_packet_side_data_remove(in->side_data,
> &in->side_data_elems,
> > +                                       AV_PKT_DATA_NEW_EXTRADATA);
> > +            av_packet_add_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
> > +                                    ctx->par_out->extradata,
> > +                                    ctx->par_out->extradata_size);
> > +        }
> >     }
> >
> >     /* nothing to filter */
> > --
> > 2.49.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".
>
>
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
  2025-04-24  5:18   ` jie jiang
@ 2025-04-24  6:41     ` Zhao Zhili
  0 siblings, 0 replies; 11+ messages in thread
From: Zhao Zhili @ 2025-04-24  6:41 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> On Apr 24, 2025, at 13:18, jie jiang <jiangjie618@gmail.com> wrote:
> 
> fmpeg -i rtmp://xxx/live/xxx -bsf:v
> "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -
> 
> For h264_mp4toannexb, input 264 is avcc,  output is annexb. and for
> h264_metadata, both input and output are annexb.
> If the sps/pps header is updated,  flvdec module will insert
> AV_PKT_DATA_NEW_EXTRADATA to AVPacket and is this format of header is avcc.
> In cbs_bsf_update_side_data function, it will find
> AV_PKT_DATA_NEW_EXTRADATA and this extradata is avcc, that will cause
> h264_metadata to judge that its input is also avcc, but  its input  is from
> h264_mp4toannexb. Using the logic of parsing avcc to parse annexb is
> problematic.
> 
> 

So only remove AV_PKT_DATA_NEW_EXTRADATA can fix the issue?
And the error is from bsf or decoder after h264_mp4toannexb, not h264_mp4toannexb itself, right?

Please share a sample file and add a fate test.

> 
> 
> On Thu, Apr 24, 2025 at 12:19 PM Zhao Zhili <quinkblack@foxmail.com> wrote:
> 
>> 
>> 
>>> On Apr 24, 2025, at 11:38, jiangjie <jiangjie618@gmail.com> wrote:
>>> 
>>> if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter will
>> return an error.
>>> ffmpeg -i rtmp://xxx/live/xxx -bsf:v
>> "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -
>> 
>> H.264 in FLV is avcc. Which service provider use annexb in FLV?
>> 
>> And I don’t think it makes sense to use AV_PKT_DATA_NEW_EXTRADATA together
>> with annexb.
>> 
>>> ---
>>> libavcodec/bsf/h264_mp4toannexb.c | 26 ++++++++++++++++++++------
>>> 1 file changed, 20 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/libavcodec/bsf/h264_mp4toannexb.c
>> b/libavcodec/bsf/h264_mp4toannexb.c
>>> index dda064287e..4cd002d166 100644
>>> --- a/libavcodec/bsf/h264_mp4toannexb.c
>>> +++ b/libavcodec/bsf/h264_mp4toannexb.c
>>> @@ -252,14 +252,21 @@ static int
>> h264_mp4toannexb_filter_ps(H264BSFContext *s,
>>>    return 0;
>>> }
>>> 
>>> +static int is_annexb(const uint8_t *extradata, int extra_size) {
>>> +    if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) ||
>>> +        (extra_size >= 4 && AV_RB32(extradata) == 1)) {
>>> +        return 1;
>>> +    } else {
>>> +        return 0;
>>> +    }
>>> +}
>>> +
>>> static int h264_mp4toannexb_init(AVBSFContext *ctx)
>>> {
>>>    int extra_size = ctx->par_in->extradata_size;
>>> 
>>>    /* retrieve sps and pps NAL units from extradata */
>>> -    if (!extra_size                                               ||
>>> -        (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) ||
>>> -        (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
>>> +    if (is_annexb(ctx->par_in->extradata, extra_size)) {
>>>        av_log(ctx, AV_LOG_VERBOSE,
>>>               "The input looks like it is Annex B already\n");
>>>    } else if (extra_size >= 7) {
>>> @@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext
>> *ctx, AVPacket *opkt)
>>>    extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
>>>                                        &extradata_size);
>>>    if (extradata) {
>>> -        ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
>>> -        if (ret < 0)
>>> -            goto fail;
>>> +        if (!is_annexb(extradata, extradata_size) && extradata_size >=
>> 7) {
>>> +            ret = h264_extradata_to_annexb(ctx, extradata,
>> extradata_size);
>>> +            if (ret < 0)
>>> +                goto fail;
>>> +            av_packet_side_data_remove(in->side_data,
>> &in->side_data_elems,
>>> +                                       AV_PKT_DATA_NEW_EXTRADATA);
>>> +            av_packet_add_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
>>> +                                    ctx->par_out->extradata,
>>> +                                    ctx->par_out->extradata_size);
>>> +        }
>>>    }
>>> 
>>>    /* nothing to filter */
>>> --
>>> 2.49.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".
>> 
>> 
> _______________________________________________
> 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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
  2025-04-24  3:38 [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc jiangjie
  2025-04-24  4:19 ` Zhao Zhili
@ 2025-04-24 12:42 ` jiangjie
  2025-04-24 13:04   ` jie jiang
  2025-04-24 14:20   ` Zhao Zhili
  1 sibling, 2 replies; 11+ messages in thread
From: jiangjie @ 2025-04-24 12:42 UTC (permalink / raw)
  To: ffmpeg-devel, quinkblack; +Cc: jiangjie618

if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter will return an error.
ffmpeg -i rtmp://xxx/live/xxx -bsf:v "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -
---
 libavcodec/bsf/h264_mp4toannexb.c             | 26 ++++++++++++++-----
 tests/fate/h264.mak                           |  3 +++
 .../fate/h264-bsf-mp4toannexb-h264_metadata   | 12 +++++++++
 3 files changed, 35 insertions(+), 6 deletions(-)
 create mode 100644 tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata

diff --git a/libavcodec/bsf/h264_mp4toannexb.c b/libavcodec/bsf/h264_mp4toannexb.c
index dda064287e..42a5a274bf 100644
--- a/libavcodec/bsf/h264_mp4toannexb.c
+++ b/libavcodec/bsf/h264_mp4toannexb.c
@@ -252,14 +252,21 @@ static int h264_mp4toannexb_filter_ps(H264BSFContext *s,
     return 0;
 }
 
+static int is_annexb(const uint8_t *extradata, int extra_size) {
+    if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) ||
+        (extra_size >= 4 && AV_RB32(extradata) == 1)) {
+        return 1;
+    } else {
+        return 0;
+    }
+}
+
 static int h264_mp4toannexb_init(AVBSFContext *ctx)
 {
     int extra_size = ctx->par_in->extradata_size;
 
     /* retrieve sps and pps NAL units from extradata */
-    if (!extra_size                                               ||
-        (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) ||
-        (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
+    if (is_annexb(ctx->par_in->extradata, extra_size)) {
         av_log(ctx, AV_LOG_VERBOSE,
                "The input looks like it is Annex B already\n");
     } else if (extra_size >= 7) {
@@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt)
     extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
                                         &extradata_size);
     if (extradata) {
-        ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
-        if (ret < 0)
-            goto fail;
+        if (!is_annexb(extradata, extradata_size) && extradata_size >= 7) {
+            ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
+            if (ret < 0)
+                goto fail;
+            av_packet_side_data_remove(in->side_data, &in->side_data_elems,
+                                       AV_PKT_DATA_NEW_EXTRADATA);
+            uint8_t *data = av_packet_new_side_data(
+                in, AV_PKT_DATA_NEW_EXTRADATA, ctx->par_out->extradata_size);
+            memcpy(data, ctx->par_out->extradata, ctx->par_out->extradata_size);
+        }
     }
 
     /* nothing to filter */
diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
index 00b153cc93..7b3e865955 100644
--- a/tests/fate/h264.mak
+++ b/tests/fate/h264.mak
@@ -231,6 +231,7 @@ FATE_H264-$(call DEMMUX, MOV, H264, H264_MP4TOANNEXB_BSF) += fate-h264-bsf-mp4to
                                                              fate-h264-bsf-mp4toannexb-2 \
                                                              fate-h264_mp4toannexb_ticket5927 \
                                                              fate-h264_mp4toannexb_ticket5927_2 \
+                                                             fate-h264-bsf-mp4toannexb-h264_metadata \
 
 FATE_H264-$(call DEMMUX, H264, MOV, DTS2PTS_BSF) += fate-h264-bsf-dts2pts
 
@@ -441,6 +442,8 @@ fate-h264-bsf-mp4toannexb-2:                      CMD = md5 -i $(TARGET_SAMPLES)
 fate-h264-bsf-mp4toannexb-2:                      CMP = oneline
 fate-h264-bsf-mp4toannexb-2:                      REF = cffcfa6a2d0b58c9de1f5785f099f41d
 fate-h264-bsf-mp4toannexb-new-extradata:          CMD = stream_remux mov $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov "" h264 "-map 0:v"
+fate-h264-bsf-mp4toannexb-h264_metadata:          CMD = transcode mov $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov \
+                                                        h264 "-c:v copy -bsf:v h264_mp4toannexb,h264_metadata=aud=remove" "-c:v copy"
 fate-h264-bsf-dts2pts:                            CMD = transcode "h264" $(TARGET_SAMPLES)/h264-conformance/CAPAMA3_Sand_F.264 \
                                                         mov "-c:v copy -bsf:v dts2pts -frames:v 50" "-c:v copy"
 fate-h264_mp4toannexb_ticket5927:                 CMD = transcode "mp4" $(TARGET_SAMPLES)/h264/thezerotheorem-cut.mp4 \
diff --git a/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
new file mode 100644
index 0000000000..d3ba7f1886
--- /dev/null
+++ b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
@@ -0,0 +1,12 @@
+a48f175bf3458410d1dd1907682b92d3 *tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
+4642 tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
+#extradata 0:       38, 0x84d70914
+#tb 0: 1/1200000
+#media_type 0: video
+#codec_id 0: h264
+#dimensions 0: 256x128
+#sar 0: 1/1
+0,     -96000, -9223372036854775808,    48000,     2331, 0xc97509cb
+0,     -48000, -9223372036854775808,    48000,       64, 0x6ad91f4e, F=0x0
+0,          0, -9223372036854775808,    48000,     2203, 0xe376d2a9
+0,      48000, -9223372036854775808,    48000,       44, 0x687a10ea, F=0x0
-- 
2.49.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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
  2025-04-24 12:42 ` jiangjie
@ 2025-04-24 13:04   ` jie jiang
  2025-04-24 14:03     ` Zhao Zhili
  2025-04-24 14:20   ` Zhao Zhili
  1 sibling, 1 reply; 11+ messages in thread
From: jie jiang @ 2025-04-24 13:04 UTC (permalink / raw)
  To: ffmpeg-devel, quinkblack

I resubmitted it according to your suggestion.
please check.
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20250424124250.71553-1-jiangjie618@gmail.com/


So only remove AV_PKT_DATA_NEW_EXTRADATA can fix the issue?
answer:  yes,  but AV_PKT_DATA_NEW_EXTRADATA is used for event
notification, it is usefull.

And the error is from bsf or decoder after h264_mp4toannexb, not
h264_mp4toannexb itself, right?
answer:  yes

i think AV_PKT_DATA_NEW_EXTRADATA and output stream should be the same
format, both avvc or  annexb.




On Thu, Apr 24, 2025 at 8:43 PM jiangjie <jiangjie618@gmail.com> wrote:

> if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter will
> return an error.
> ffmpeg -i rtmp://xxx/live/xxx -bsf:v
> "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -
> ---
>  libavcodec/bsf/h264_mp4toannexb.c             | 26 ++++++++++++++-----
>  tests/fate/h264.mak                           |  3 +++
>  .../fate/h264-bsf-mp4toannexb-h264_metadata   | 12 +++++++++
>  3 files changed, 35 insertions(+), 6 deletions(-)
>  create mode 100644 tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
>
> diff --git a/libavcodec/bsf/h264_mp4toannexb.c
> b/libavcodec/bsf/h264_mp4toannexb.c
> index dda064287e..42a5a274bf 100644
> --- a/libavcodec/bsf/h264_mp4toannexb.c
> +++ b/libavcodec/bsf/h264_mp4toannexb.c
> @@ -252,14 +252,21 @@ static int h264_mp4toannexb_filter_ps(H264BSFContext
> *s,
>      return 0;
>  }
>
> +static int is_annexb(const uint8_t *extradata, int extra_size) {
> +    if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) ||
> +        (extra_size >= 4 && AV_RB32(extradata) == 1)) {
> +        return 1;
> +    } else {
> +        return 0;
> +    }
> +}
> +
>  static int h264_mp4toannexb_init(AVBSFContext *ctx)
>  {
>      int extra_size = ctx->par_in->extradata_size;
>
>      /* retrieve sps and pps NAL units from extradata */
> -    if (!extra_size                                               ||
> -        (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) ||
> -        (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
> +    if (is_annexb(ctx->par_in->extradata, extra_size)) {
>          av_log(ctx, AV_LOG_VERBOSE,
>                 "The input looks like it is Annex B already\n");
>      } else if (extra_size >= 7) {
> @@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx,
> AVPacket *opkt)
>      extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
>                                          &extradata_size);
>      if (extradata) {
> -        ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
> -        if (ret < 0)
> -            goto fail;
> +        if (!is_annexb(extradata, extradata_size) && extradata_size >= 7)
> {
> +            ret = h264_extradata_to_annexb(ctx, extradata,
> extradata_size);
> +            if (ret < 0)
> +                goto fail;
> +            av_packet_side_data_remove(in->side_data,
> &in->side_data_elems,
> +                                       AV_PKT_DATA_NEW_EXTRADATA);
> +            uint8_t *data = av_packet_new_side_data(
> +                in, AV_PKT_DATA_NEW_EXTRADATA,
> ctx->par_out->extradata_size);
> +            memcpy(data, ctx->par_out->extradata,
> ctx->par_out->extradata_size);
> +        }
>      }
>
>      /* nothing to filter */
> diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
> index 00b153cc93..7b3e865955 100644
> --- a/tests/fate/h264.mak
> +++ b/tests/fate/h264.mak
> @@ -231,6 +231,7 @@ FATE_H264-$(call DEMMUX, MOV, H264,
> H264_MP4TOANNEXB_BSF) += fate-h264-bsf-mp4to
>
> fate-h264-bsf-mp4toannexb-2 \
>
> fate-h264_mp4toannexb_ticket5927 \
>
> fate-h264_mp4toannexb_ticket5927_2 \
> +
>  fate-h264-bsf-mp4toannexb-h264_metadata \
>
>  FATE_H264-$(call DEMMUX, H264, MOV, DTS2PTS_BSF) += fate-h264-bsf-dts2pts
>
> @@ -441,6 +442,8 @@ fate-h264-bsf-mp4toannexb-2:                      CMD
> = md5 -i $(TARGET_SAMPLES)
>  fate-h264-bsf-mp4toannexb-2:                      CMP = oneline
>  fate-h264-bsf-mp4toannexb-2:                      REF =
> cffcfa6a2d0b58c9de1f5785f099f41d
>  fate-h264-bsf-mp4toannexb-new-extradata:          CMD = stream_remux mov
> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov "" h264 "-map 0:v"
> +fate-h264-bsf-mp4toannexb-h264_metadata:          CMD = transcode mov
> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov \
> +                                                        h264 "-c:v copy
> -bsf:v h264_mp4toannexb,h264_metadata=aud=remove" "-c:v copy"
>  fate-h264-bsf-dts2pts:                            CMD = transcode "h264"
> $(TARGET_SAMPLES)/h264-conformance/CAPAMA3_Sand_F.264 \
>                                                          mov "-c:v copy
> -bsf:v dts2pts -frames:v 50" "-c:v copy"
>  fate-h264_mp4toannexb_ticket5927:                 CMD = transcode "mp4"
> $(TARGET_SAMPLES)/h264/thezerotheorem-cut.mp4 \
> diff --git a/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> new file mode 100644
> index 0000000000..d3ba7f1886
> --- /dev/null
> +++ b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> @@ -0,0 +1,12 @@
> +a48f175bf3458410d1dd1907682b92d3
> *tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
> +4642 tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
> +#extradata 0:       38, 0x84d70914
> +#tb 0: 1/1200000
> +#media_type 0: video
> +#codec_id 0: h264
> +#dimensions 0: 256x128
> +#sar 0: 1/1
> +0,     -96000, -9223372036854775808,    48000,     2331, 0xc97509cb
> +0,     -48000, -9223372036854775808,    48000,       64, 0x6ad91f4e, F=0x0
> +0,          0, -9223372036854775808,    48000,     2203, 0xe376d2a9
> +0,      48000, -9223372036854775808,    48000,       44, 0x687a10ea, F=0x0
> --
> 2.49.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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
  2025-04-24 13:04   ` jie jiang
@ 2025-04-24 14:03     ` Zhao Zhili
  0 siblings, 0 replies; 11+ messages in thread
From: Zhao Zhili @ 2025-04-24 14:03 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> On Apr 24, 2025, at 21:04, jie jiang <jiangjie618@gmail.com> wrote:
> 
> I resubmitted it according to your suggestion.
> please check.
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20250424124250.71553-1-jiangjie618@gmail.com/
> 
> 
> So only remove AV_PKT_DATA_NEW_EXTRADATA can fix the issue?
> answer:  yes,  but AV_PKT_DATA_NEW_EXTRADATA is used for event
> notification, it is usefull.

This isn’t what I mean. I mean the patch should only remove AV_PKT_DATA_NEW_EXTRADATA from packet,
not updating ctx->par_out->extradata.

> 
> And the error is from bsf or decoder after h264_mp4toannexb, not
> h264_mp4toannexb itself, right?
> answer:  yes
> 
> i think AV_PKT_DATA_NEW_EXTRADATA and output stream should be the same
> format, both avvc or  annexb.
> 
> 
> 
> 
> On Thu, Apr 24, 2025 at 8:43 PM jiangjie <jiangjie618@gmail.com> wrote:
> 
>> if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter will
>> return an error.
>> ffmpeg -i rtmp://xxx/live/xxx -bsf:v
>> "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -
>> ---
>> libavcodec/bsf/h264_mp4toannexb.c             | 26 ++++++++++++++-----
>> tests/fate/h264.mak                           |  3 +++
>> .../fate/h264-bsf-mp4toannexb-h264_metadata   | 12 +++++++++
>> 3 files changed, 35 insertions(+), 6 deletions(-)
>> create mode 100644 tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
>> 
>> diff --git a/libavcodec/bsf/h264_mp4toannexb.c
>> b/libavcodec/bsf/h264_mp4toannexb.c
>> index dda064287e..42a5a274bf 100644
>> --- a/libavcodec/bsf/h264_mp4toannexb.c
>> +++ b/libavcodec/bsf/h264_mp4toannexb.c
>> @@ -252,14 +252,21 @@ static int h264_mp4toannexb_filter_ps(H264BSFContext
>> *s,
>>     return 0;
>> }
>> 
>> +static int is_annexb(const uint8_t *extradata, int extra_size) {
>> +    if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) ||
>> +        (extra_size >= 4 && AV_RB32(extradata) == 1)) {
>> +        return 1;
>> +    } else {
>> +        return 0;
>> +    }
>> +}
>> +
>> static int h264_mp4toannexb_init(AVBSFContext *ctx)
>> {
>>     int extra_size = ctx->par_in->extradata_size;
>> 
>>     /* retrieve sps and pps NAL units from extradata */
>> -    if (!extra_size                                               ||
>> -        (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) ||
>> -        (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
>> +    if (is_annexb(ctx->par_in->extradata, extra_size)) {
>>         av_log(ctx, AV_LOG_VERBOSE,
>>                "The input looks like it is Annex B already\n");
>>     } else if (extra_size >= 7) {
>> @@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx,
>> AVPacket *opkt)
>>     extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
>>                                         &extradata_size);
>>     if (extradata) {
>> -        ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
>> -        if (ret < 0)
>> -            goto fail;
>> +        if (!is_annexb(extradata, extradata_size) && extradata_size >= 7)
>> {
>> +            ret = h264_extradata_to_annexb(ctx, extradata,
>> extradata_size);
>> +            if (ret < 0)
>> +                goto fail;
>> +            av_packet_side_data_remove(in->side_data,
>> &in->side_data_elems,
>> +                                       AV_PKT_DATA_NEW_EXTRADATA);
>> +            uint8_t *data = av_packet_new_side_data(
>> +                in, AV_PKT_DATA_NEW_EXTRADATA,
>> ctx->par_out->extradata_size);
>> +            memcpy(data, ctx->par_out->extradata,
>> ctx->par_out->extradata_size);
>> +        }
>>     }
>> 
>>     /* nothing to filter */
>> diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
>> index 00b153cc93..7b3e865955 100644
>> --- a/tests/fate/h264.mak
>> +++ b/tests/fate/h264.mak
>> @@ -231,6 +231,7 @@ FATE_H264-$(call DEMMUX, MOV, H264,
>> H264_MP4TOANNEXB_BSF) += fate-h264-bsf-mp4to
>> 
>> fate-h264-bsf-mp4toannexb-2 \
>> 
>> fate-h264_mp4toannexb_ticket5927 \
>> 
>> fate-h264_mp4toannexb_ticket5927_2 \
>> +
>> fate-h264-bsf-mp4toannexb-h264_metadata \
>> 
>> FATE_H264-$(call DEMMUX, H264, MOV, DTS2PTS_BSF) += fate-h264-bsf-dts2pts
>> 
>> @@ -441,6 +442,8 @@ fate-h264-bsf-mp4toannexb-2:                      CMD
>> = md5 -i $(TARGET_SAMPLES)
>> fate-h264-bsf-mp4toannexb-2:                      CMP = oneline
>> fate-h264-bsf-mp4toannexb-2:                      REF =
>> cffcfa6a2d0b58c9de1f5785f099f41d
>> fate-h264-bsf-mp4toannexb-new-extradata:          CMD = stream_remux mov
>> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov "" h264 "-map 0:v"
>> +fate-h264-bsf-mp4toannexb-h264_metadata:          CMD = transcode mov
>> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov \
>> +                                                        h264 "-c:v copy
>> -bsf:v h264_mp4toannexb,h264_metadata=aud=remove" "-c:v copy"
>> fate-h264-bsf-dts2pts:                            CMD = transcode "h264"
>> $(TARGET_SAMPLES)/h264-conformance/CAPAMA3_Sand_F.264 \
>>                                                         mov "-c:v copy
>> -bsf:v dts2pts -frames:v 50" "-c:v copy"
>> fate-h264_mp4toannexb_ticket5927:                 CMD = transcode "mp4"
>> $(TARGET_SAMPLES)/h264/thezerotheorem-cut.mp4 \
>> diff --git a/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
>> b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
>> new file mode 100644
>> index 0000000000..d3ba7f1886
>> --- /dev/null
>> +++ b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
>> @@ -0,0 +1,12 @@
>> +a48f175bf3458410d1dd1907682b92d3
>> *tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
>> +4642 tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
>> +#extradata 0:       38, 0x84d70914
>> +#tb 0: 1/1200000
>> +#media_type 0: video
>> +#codec_id 0: h264
>> +#dimensions 0: 256x128
>> +#sar 0: 1/1
>> +0,     -96000, -9223372036854775808,    48000,     2331, 0xc97509cb
>> +0,     -48000, -9223372036854775808,    48000,       64, 0x6ad91f4e, F=0x0
>> +0,          0, -9223372036854775808,    48000,     2203, 0xe376d2a9
>> +0,      48000, -9223372036854775808,    48000,       44, 0x687a10ea, F=0x0
>> --
>> 2.49.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".

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
  2025-04-24 12:42 ` jiangjie
  2025-04-24 13:04   ` jie jiang
@ 2025-04-24 14:20   ` Zhao Zhili
  2025-04-24 14:53     ` jie jiang
  1 sibling, 1 reply; 11+ messages in thread
From: Zhao Zhili @ 2025-04-24 14:20 UTC (permalink / raw)
  To: ffmpeg-devel



> On Apr 24, 2025, at 20:42, jiangjie <jiangjie618@gmail.com> wrote:
> 
> if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter will return an error.
> ffmpeg -i rtmp://xxx/live/xxx -bsf:v "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -

The subject and commit message is confusing.

’this’ isn’t a bitstream filter, it’s a bsf chain. And what report error is h264_metadata, not h264_mp4toannexb.

The subject can be: Remove NEW_EXTRADATA sidedata

And commit message can be:

We don’t support mixed bitstream format. For example, h264_metadata report error in this case


> ---
> libavcodec/bsf/h264_mp4toannexb.c             | 26 ++++++++++++++-----
> tests/fate/h264.mak                           |  3 +++
> .../fate/h264-bsf-mp4toannexb-h264_metadata   | 12 +++++++++
> 3 files changed, 35 insertions(+), 6 deletions(-)
> create mode 100644 tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> 
> diff --git a/libavcodec/bsf/h264_mp4toannexb.c b/libavcodec/bsf/h264_mp4toannexb.c
> index dda064287e..42a5a274bf 100644
> --- a/libavcodec/bsf/h264_mp4toannexb.c
> +++ b/libavcodec/bsf/h264_mp4toannexb.c
> @@ -252,14 +252,21 @@ static int h264_mp4toannexb_filter_ps(H264BSFContext *s,
>     return 0;
> }
> 
> +static int is_annexb(const uint8_t *extradata, int extra_size) {
> +    if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) ||
> +        (extra_size >= 4 && AV_RB32(extradata) == 1)) {
> +        return 1;
> +    } else {
> +        return 0;
> +    }
> +}
> +
> static int h264_mp4toannexb_init(AVBSFContext *ctx)
> {
>     int extra_size = ctx->par_in->extradata_size;
> 
>     /* retrieve sps and pps NAL units from extradata */
> -    if (!extra_size                                               ||
> -        (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) ||
> -        (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
> +    if (is_annexb(ctx->par_in->extradata, extra_size)) {
>         av_log(ctx, AV_LOG_VERBOSE,
>                "The input looks like it is Annex B already\n");
>     } else if (extra_size >= 7) {
> @@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt)
>     extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
>                                         &extradata_size);
>     if (extradata) {
> -        ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
> -        if (ret < 0)
> -            goto fail;
> +        if (!is_annexb(extradata, extradata_size) && extradata_size >= 7) {
> +            ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
> +            if (ret < 0)
> +                goto fail;
> +            av_packet_side_data_remove(in->side_data, &in->side_data_elems,
> +                                       AV_PKT_DATA_NEW_EXTRADATA);
> +            uint8_t *data = av_packet_new_side_data(
> +                in, AV_PKT_DATA_NEW_EXTRADATA, ctx->par_out->extradata_size);
> +            memcpy(data, ctx->par_out->extradata, ctx->par_out->extradata_size);
> +        }

Remove adding AV_PKT_DATA_NEW_EXTRADATA. There’re in-band PS now.

There is another case which not belonging to this patch. With AV_PKT_DATA_NEW_EXTRADATA
in annexb format, it should be inserted as in-band PS and also be removed after. 

>     }
> 
>     /* nothing to filter */
> diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
> index 00b153cc93..7b3e865955 100644
> --- a/tests/fate/h264.mak
> +++ b/tests/fate/h264.mak
> @@ -231,6 +231,7 @@ FATE_H264-$(call DEMMUX, MOV, H264, H264_MP4TOANNEXB_BSF) += fate-h264-bsf-mp4to
>                                                              fate-h264-bsf-mp4toannexb-2 \
>                                                              fate-h264_mp4toannexb_ticket5927 \
>                                                              fate-h264_mp4toannexb_ticket5927_2 \
> +                                                             fate-h264-bsf-mp4toannexb-h264_metadata \
> 
> FATE_H264-$(call DEMMUX, H264, MOV, DTS2PTS_BSF) += fate-h264-bsf-dts2pts
> 
> @@ -441,6 +442,8 @@ fate-h264-bsf-mp4toannexb-2:                      CMD = md5 -i $(TARGET_SAMPLES)
> fate-h264-bsf-mp4toannexb-2:                      CMP = oneline
> fate-h264-bsf-mp4toannexb-2:                      REF = cffcfa6a2d0b58c9de1f5785f099f41d
> fate-h264-bsf-mp4toannexb-new-extradata:          CMD = stream_remux mov $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov "" h264 "-map 0:v"
> +fate-h264-bsf-mp4toannexb-h264_metadata:          CMD = transcode mov $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov \
> +                                                        h264 "-c:v copy -bsf:v h264_mp4toannexb,h264_metadata=aud=remove" "-c:v copy"
> fate-h264-bsf-dts2pts:                            CMD = transcode "h264" $(TARGET_SAMPLES)/h264-conformance/CAPAMA3_Sand_F.264 \
>                                                         mov "-c:v copy -bsf:v dts2pts -frames:v 50" "-c:v copy"
> fate-h264_mp4toannexb_ticket5927:                 CMD = transcode "mp4" $(TARGET_SAMPLES)/h264/thezerotheorem-cut.mp4 \
> diff --git a/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> new file mode 100644
> index 0000000000..d3ba7f1886
> --- /dev/null
> +++ b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> @@ -0,0 +1,12 @@
> +a48f175bf3458410d1dd1907682b92d3 *tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
> +4642 tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
> +#extradata 0:       38, 0x84d70914
> +#tb 0: 1/1200000
> +#media_type 0: video
> +#codec_id 0: h264
> +#dimensions 0: 256x128
> +#sar 0: 1/1
> +0,     -96000, -9223372036854775808,    48000,     2331, 0xc97509cb
> +0,     -48000, -9223372036854775808,    48000,       64, 0x6ad91f4e, F=0x0
> +0,          0, -9223372036854775808,    48000,     2203, 0xe376d2a9
> +0,      48000, -9223372036854775808,    48000,       44, 0x687a10ea, F=0x0
> -- 
> 2.49.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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
  2025-04-24 14:20   ` Zhao Zhili
@ 2025-04-24 14:53     ` jie jiang
  2025-04-24 17:23       ` Zhao Zhili
  0 siblings, 1 reply; 11+ messages in thread
From: jie jiang @ 2025-04-24 14:53 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

ctx->par_out->extradata isn't  updated by this commit.  Only NEW_EXTRADATA
is updated (avvc to annexb).

You suggest the patch should only remove AV_PKT_DATA_NEW_EXTRADATA from
packet, but AV_PKT_DATA_NEW_EXTRADATA is used to notify the codec or the
format that the extradata buffer was changed and the receiving side should
act upon it appropriately. So i think NEW_EXTRADATA should be reserved.

If you think we don't need support mixed bitstream format.  maybe this
patch that i commit should be rejected.

On Thu, Apr 24, 2025 at 10:21 PM Zhao Zhili <
quinkblack-at-foxmail.com@ffmpeg.org> wrote:

>
>
> > On Apr 24, 2025, at 20:42, jiangjie <jiangjie618@gmail.com> wrote:
> >
> > if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter will
> return an error.
> > ffmpeg -i rtmp://xxx/live/xxx -bsf:v
> "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -
>
> The subject and commit message is confusing.
>
> ’this’ isn’t a bitstream filter, it’s a bsf chain. And what report error
> is h264_metadata, not h264_mp4toannexb.
>
> The subject can be: Remove NEW_EXTRADATA sidedata
>
> And commit message can be:
>
> We don’t support mixed bitstream format. For example, h264_metadata report
> error in this case
>
>
> > ---
> > libavcodec/bsf/h264_mp4toannexb.c             | 26 ++++++++++++++-----
> > tests/fate/h264.mak                           |  3 +++
> > .../fate/h264-bsf-mp4toannexb-h264_metadata   | 12 +++++++++
> > 3 files changed, 35 insertions(+), 6 deletions(-)
> > create mode 100644 tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> >
> > diff --git a/libavcodec/bsf/h264_mp4toannexb.c
> b/libavcodec/bsf/h264_mp4toannexb.c
> > index dda064287e..42a5a274bf 100644
> > --- a/libavcodec/bsf/h264_mp4toannexb.c
> > +++ b/libavcodec/bsf/h264_mp4toannexb.c
> > @@ -252,14 +252,21 @@ static int
> h264_mp4toannexb_filter_ps(H264BSFContext *s,
> >     return 0;
> > }
> >
> > +static int is_annexb(const uint8_t *extradata, int extra_size) {
> > +    if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) ||
> > +        (extra_size >= 4 && AV_RB32(extradata) == 1)) {
> > +        return 1;
> > +    } else {
> > +        return 0;
> > +    }
> > +}
> > +
> > static int h264_mp4toannexb_init(AVBSFContext *ctx)
> > {
> >     int extra_size = ctx->par_in->extradata_size;
> >
> >     /* retrieve sps and pps NAL units from extradata */
> > -    if (!extra_size                                               ||
> > -        (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) ||
> > -        (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
> > +    if (is_annexb(ctx->par_in->extradata, extra_size)) {
> >         av_log(ctx, AV_LOG_VERBOSE,
> >                "The input looks like it is Annex B already\n");
> >     } else if (extra_size >= 7) {
> > @@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext
> *ctx, AVPacket *opkt)
> >     extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
> >                                         &extradata_size);
> >     if (extradata) {
> > -        ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
> > -        if (ret < 0)
> > -            goto fail;
> > +        if (!is_annexb(extradata, extradata_size) && extradata_size >=
> 7) {
> > +            ret = h264_extradata_to_annexb(ctx, extradata,
> extradata_size);
> > +            if (ret < 0)
> > +                goto fail;
> > +            av_packet_side_data_remove(in->side_data,
> &in->side_data_elems,
> > +                                       AV_PKT_DATA_NEW_EXTRADATA);
> > +            uint8_t *data = av_packet_new_side_data(
> > +                in, AV_PKT_DATA_NEW_EXTRADATA,
> ctx->par_out->extradata_size);
> > +            memcpy(data, ctx->par_out->extradata,
> ctx->par_out->extradata_size);
> > +        }
>
> Remove adding AV_PKT_DATA_NEW_EXTRADATA. There’re in-band PS now.
>
> There is another case which not belonging to this patch. With
> AV_PKT_DATA_NEW_EXTRADATA
> in annexb format, it should be inserted as in-band PS and also be removed
> after.
>
> >     }
> >
> >     /* nothing to filter */
> > diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
> > index 00b153cc93..7b3e865955 100644
> > --- a/tests/fate/h264.mak
> > +++ b/tests/fate/h264.mak
> > @@ -231,6 +231,7 @@ FATE_H264-$(call DEMMUX, MOV, H264,
> H264_MP4TOANNEXB_BSF) += fate-h264-bsf-mp4to
> >
> fate-h264-bsf-mp4toannexb-2 \
> >
> fate-h264_mp4toannexb_ticket5927 \
> >
> fate-h264_mp4toannexb_ticket5927_2 \
> > +
>  fate-h264-bsf-mp4toannexb-h264_metadata \
> >
> > FATE_H264-$(call DEMMUX, H264, MOV, DTS2PTS_BSF) += fate-h264-bsf-dts2pts
> >
> > @@ -441,6 +442,8 @@ fate-h264-bsf-mp4toannexb-2:
> CMD = md5 -i $(TARGET_SAMPLES)
> > fate-h264-bsf-mp4toannexb-2:                      CMP = oneline
> > fate-h264-bsf-mp4toannexb-2:                      REF =
> cffcfa6a2d0b58c9de1f5785f099f41d
> > fate-h264-bsf-mp4toannexb-new-extradata:          CMD = stream_remux mov
> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov "" h264 "-map 0:v"
> > +fate-h264-bsf-mp4toannexb-h264_metadata:          CMD = transcode mov
> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov \
> > +                                                        h264 "-c:v copy
> -bsf:v h264_mp4toannexb,h264_metadata=aud=remove" "-c:v copy"
> > fate-h264-bsf-dts2pts:                            CMD = transcode "h264"
> $(TARGET_SAMPLES)/h264-conformance/CAPAMA3_Sand_F.264 \
> >                                                         mov "-c:v copy
> -bsf:v dts2pts -frames:v 50" "-c:v copy"
> > fate-h264_mp4toannexb_ticket5927:                 CMD = transcode "mp4"
> $(TARGET_SAMPLES)/h264/thezerotheorem-cut.mp4 \
> > diff --git a/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> > new file mode 100644
> > index 0000000000..d3ba7f1886
> > --- /dev/null
> > +++ b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> > @@ -0,0 +1,12 @@
> > +a48f175bf3458410d1dd1907682b92d3
> *tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
> > +4642 tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
> > +#extradata 0:       38, 0x84d70914
> > +#tb 0: 1/1200000
> > +#media_type 0: video
> > +#codec_id 0: h264
> > +#dimensions 0: 256x128
> > +#sar 0: 1/1
> > +0,     -96000, -9223372036854775808,    48000,     2331, 0xc97509cb
> > +0,     -48000, -9223372036854775808,    48000,       64, 0x6ad91f4e,
> F=0x0
> > +0,          0, -9223372036854775808,    48000,     2203, 0xe376d2a9
> > +0,      48000, -9223372036854775808,    48000,       44, 0x687a10ea,
> F=0x0
> > --
> > 2.49.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".
>
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
  2025-04-24 14:53     ` jie jiang
@ 2025-04-24 17:23       ` Zhao Zhili
  2025-04-25  2:41         ` jie jiang
  0 siblings, 1 reply; 11+ messages in thread
From: Zhao Zhili @ 2025-04-24 17:23 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Apr 24, 2025, at 22:53, jie jiang <jiangjie618@gmail.com> wrote:
> 
> ctx->par_out->extradata isn't  updated by this commit.  Only NEW_EXTRADATA
> is updated (avvc to annexb).
> 
> You suggest the patch should only remove AV_PKT_DATA_NEW_EXTRADATA from
> packet, but AV_PKT_DATA_NEW_EXTRADATA is used to notify the codec or the
> format that the extradata buffer was changed and the receiving side should
> act upon it appropriately. So i think NEW_EXTRADATA should be reserved.

h264_mp4toannexb is supposed to convert out-of-band PS in avcc format to in-band
annex format, for decoder and for muxer like mpegts. It can be confusing to keep
another group of duplicated PS in side data.

And we have extract_extradata bsf for any case NEW_EXTRADATA in side data is needed.

> 
> If you think we don't need support mixed bitstream format.  maybe this
> patch that i commit should be rejected.

We don’t support mixed bitstream format. The issue is totally valid and needs to be fixed.
Here is my patch:

https://ffmpeg.org/pipermail/ffmpeg-devel/2025-April/342723.html

> 
> On Thu, Apr 24, 2025 at 10:21 PM Zhao Zhili <
> quinkblack-at-foxmail.com@ffmpeg.org> wrote:
> 
>> 
>> 
>>> On Apr 24, 2025, at 20:42, jiangjie <jiangjie618@gmail.com> wrote:
>>> 
>>> if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter will
>> return an error.
>>> ffmpeg -i rtmp://xxx/live/xxx -bsf:v
>> "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -
>> 
>> The subject and commit message is confusing.
>> 
>> ’this’ isn’t a bitstream filter, it’s a bsf chain. And what report error
>> is h264_metadata, not h264_mp4toannexb.
>> 
>> The subject can be: Remove NEW_EXTRADATA sidedata
>> 
>> And commit message can be:
>> 
>> We don’t support mixed bitstream format. For example, h264_metadata report
>> error in this case
>> 
>> 
>>> ---
>>> libavcodec/bsf/h264_mp4toannexb.c             | 26 ++++++++++++++-----
>>> tests/fate/h264.mak                           |  3 +++
>>> .../fate/h264-bsf-mp4toannexb-h264_metadata   | 12 +++++++++
>>> 3 files changed, 35 insertions(+), 6 deletions(-)
>>> create mode 100644 tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
>>> 
>>> diff --git a/libavcodec/bsf/h264_mp4toannexb.c
>> b/libavcodec/bsf/h264_mp4toannexb.c
>>> index dda064287e..42a5a274bf 100644
>>> --- a/libavcodec/bsf/h264_mp4toannexb.c
>>> +++ b/libavcodec/bsf/h264_mp4toannexb.c
>>> @@ -252,14 +252,21 @@ static int
>> h264_mp4toannexb_filter_ps(H264BSFContext *s,
>>>    return 0;
>>> }
>>> 
>>> +static int is_annexb(const uint8_t *extradata, int extra_size) {
>>> +    if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) ||
>>> +        (extra_size >= 4 && AV_RB32(extradata) == 1)) {
>>> +        return 1;
>>> +    } else {
>>> +        return 0;
>>> +    }
>>> +}
>>> +
>>> static int h264_mp4toannexb_init(AVBSFContext *ctx)
>>> {
>>>    int extra_size = ctx->par_in->extradata_size;
>>> 
>>>    /* retrieve sps and pps NAL units from extradata */
>>> -    if (!extra_size                                               ||
>>> -        (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) ||
>>> -        (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
>>> +    if (is_annexb(ctx->par_in->extradata, extra_size)) {
>>>        av_log(ctx, AV_LOG_VERBOSE,
>>>               "The input looks like it is Annex B already\n");
>>>    } else if (extra_size >= 7) {
>>> @@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext
>> *ctx, AVPacket *opkt)
>>>    extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
>>>                                        &extradata_size);
>>>    if (extradata) {
>>> -        ret = h264_extradata_to_annexb(ctx, extradata, extradata_size);
>>> -        if (ret < 0)
>>> -            goto fail;
>>> +        if (!is_annexb(extradata, extradata_size) && extradata_size >=
>> 7) {
>>> +            ret = h264_extradata_to_annexb(ctx, extradata,
>> extradata_size);
>>> +            if (ret < 0)
>>> +                goto fail;
>>> +            av_packet_side_data_remove(in->side_data,
>> &in->side_data_elems,
>>> +                                       AV_PKT_DATA_NEW_EXTRADATA);
>>> +            uint8_t *data = av_packet_new_side_data(
>>> +                in, AV_PKT_DATA_NEW_EXTRADATA,
>> ctx->par_out->extradata_size);
>>> +            memcpy(data, ctx->par_out->extradata,
>> ctx->par_out->extradata_size);
>>> +        }
>> 
>> Remove adding AV_PKT_DATA_NEW_EXTRADATA. There’re in-band PS now.
>> 
>> There is another case which not belonging to this patch. With
>> AV_PKT_DATA_NEW_EXTRADATA
>> in annexb format, it should be inserted as in-band PS and also be removed
>> after.
>> 
>>>    }
>>> 
>>>    /* nothing to filter */
>>> diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
>>> index 00b153cc93..7b3e865955 100644
>>> --- a/tests/fate/h264.mak
>>> +++ b/tests/fate/h264.mak
>>> @@ -231,6 +231,7 @@ FATE_H264-$(call DEMMUX, MOV, H264,
>> H264_MP4TOANNEXB_BSF) += fate-h264-bsf-mp4to
>>> 
>> fate-h264-bsf-mp4toannexb-2 \
>>> 
>> fate-h264_mp4toannexb_ticket5927 \
>>> 
>> fate-h264_mp4toannexb_ticket5927_2 \
>>> +
>> fate-h264-bsf-mp4toannexb-h264_metadata \
>>> 
>>> FATE_H264-$(call DEMMUX, H264, MOV, DTS2PTS_BSF) += fate-h264-bsf-dts2pts
>>> 
>>> @@ -441,6 +442,8 @@ fate-h264-bsf-mp4toannexb-2:
>> CMD = md5 -i $(TARGET_SAMPLES)
>>> fate-h264-bsf-mp4toannexb-2:                      CMP = oneline
>>> fate-h264-bsf-mp4toannexb-2:                      REF =
>> cffcfa6a2d0b58c9de1f5785f099f41d
>>> fate-h264-bsf-mp4toannexb-new-extradata:          CMD = stream_remux mov
>> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov "" h264 "-map 0:v"
>>> +fate-h264-bsf-mp4toannexb-h264_metadata:          CMD = transcode mov
>> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov \
>>> +                                                        h264 "-c:v copy
>> -bsf:v h264_mp4toannexb,h264_metadata=aud=remove" "-c:v copy"
>>> fate-h264-bsf-dts2pts:                            CMD = transcode "h264"
>> $(TARGET_SAMPLES)/h264-conformance/CAPAMA3_Sand_F.264 \
>>>                                                        mov "-c:v copy
>> -bsf:v dts2pts -frames:v 50" "-c:v copy"
>>> fate-h264_mp4toannexb_ticket5927:                 CMD = transcode "mp4"
>> $(TARGET_SAMPLES)/h264/thezerotheorem-cut.mp4 \
>>> diff --git a/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
>> b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
>>> new file mode 100644
>>> index 0000000000..d3ba7f1886
>>> --- /dev/null
>>> +++ b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
>>> @@ -0,0 +1,12 @@
>>> +a48f175bf3458410d1dd1907682b92d3
>> *tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
>>> +4642 tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
>>> +#extradata 0:       38, 0x84d70914
>>> +#tb 0: 1/1200000
>>> +#media_type 0: video
>>> +#codec_id 0: h264
>>> +#dimensions 0: 256x128
>>> +#sar 0: 1/1
>>> +0,     -96000, -9223372036854775808,    48000,     2331, 0xc97509cb
>>> +0,     -48000, -9223372036854775808,    48000,       64, 0x6ad91f4e,
>> F=0x0
>>> +0,          0, -9223372036854775808,    48000,     2203, 0xe376d2a9
>>> +0,      48000, -9223372036854775808,    48000,       44, 0x687a10ea,
>> F=0x0
>>> --
>>> 2.49.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".
>> 
> _______________________________________________
> 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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
  2025-04-24 17:23       ` Zhao Zhili
@ 2025-04-25  2:41         ` jie jiang
  0 siblings, 0 replies; 11+ messages in thread
From: jie jiang @ 2025-04-25  2:41 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Yes, your patch solves my problem too, which is very nice.

But I still think AV_PKT_DATA_NEW_EXTRADATA is not just data, it is also an
event. As sps/pps data it can be removed,  but as event it should be
reserved.

Anyway, your patch should be merged.

On Fri, Apr 25, 2025 at 1:24 AM Zhao Zhili <
quinkblack-at-foxmail.com@ffmpeg.org> wrote:

> On Apr 24, 2025, at 22:53, jie jiang <jiangjie618@gmail.com> wrote:
> >
> > ctx->par_out->extradata isn't  updated by this commit.  Only
> NEW_EXTRADATA
> > is updated (avvc to annexb).
> >
> > You suggest the patch should only remove AV_PKT_DATA_NEW_EXTRADATA from
> > packet, but AV_PKT_DATA_NEW_EXTRADATA is used to notify the codec or the
> > format that the extradata buffer was changed and the receiving side
> should
> > act upon it appropriately. So i think NEW_EXTRADATA should be reserved.
>
> h264_mp4toannexb is supposed to convert out-of-band PS in avcc format to
> in-band
> annex format, for decoder and for muxer like mpegts. It can be confusing
> to keep
> another group of duplicated PS in side data.
>
> And we have extract_extradata bsf for any case NEW_EXTRADATA in side data
> is needed.
>
> >
> > If you think we don't need support mixed bitstream format.  maybe this
> > patch that i commit should be rejected.
>
> We don’t support mixed bitstream format. The issue is totally valid and
> needs to be fixed.
> Here is my patch:
>
> https://ffmpeg.org/pipermail/ffmpeg-devel/2025-April/342723.html
>
> >
> > On Thu, Apr 24, 2025 at 10:21 PM Zhao Zhili <
> > quinkblack-at-foxmail.com@ffmpeg.org> wrote:
> >
> >>
> >>
> >>> On Apr 24, 2025, at 20:42, jiangjie <jiangjie618@gmail.com> wrote:
> >>>
> >>> if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter
> will
> >> return an error.
> >>> ffmpeg -i rtmp://xxx/live/xxx -bsf:v
> >> "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null -
> >>
> >> The subject and commit message is confusing.
> >>
> >> ’this’ isn’t a bitstream filter, it’s a bsf chain. And what report error
> >> is h264_metadata, not h264_mp4toannexb.
> >>
> >> The subject can be: Remove NEW_EXTRADATA sidedata
> >>
> >> And commit message can be:
> >>
> >> We don’t support mixed bitstream format. For example, h264_metadata
> report
> >> error in this case
> >>
> >>
> >>> ---
> >>> libavcodec/bsf/h264_mp4toannexb.c             | 26 ++++++++++++++-----
> >>> tests/fate/h264.mak                           |  3 +++
> >>> .../fate/h264-bsf-mp4toannexb-h264_metadata   | 12 +++++++++
> >>> 3 files changed, 35 insertions(+), 6 deletions(-)
> >>> create mode 100644 tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> >>>
> >>> diff --git a/libavcodec/bsf/h264_mp4toannexb.c
> >> b/libavcodec/bsf/h264_mp4toannexb.c
> >>> index dda064287e..42a5a274bf 100644
> >>> --- a/libavcodec/bsf/h264_mp4toannexb.c
> >>> +++ b/libavcodec/bsf/h264_mp4toannexb.c
> >>> @@ -252,14 +252,21 @@ static int
> >> h264_mp4toannexb_filter_ps(H264BSFContext *s,
> >>>    return 0;
> >>> }
> >>>
> >>> +static int is_annexb(const uint8_t *extradata, int extra_size) {
> >>> +    if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) ||
> >>> +        (extra_size >= 4 && AV_RB32(extradata) == 1)) {
> >>> +        return 1;
> >>> +    } else {
> >>> +        return 0;
> >>> +    }
> >>> +}
> >>> +
> >>> static int h264_mp4toannexb_init(AVBSFContext *ctx)
> >>> {
> >>>    int extra_size = ctx->par_in->extradata_size;
> >>>
> >>>    /* retrieve sps and pps NAL units from extradata */
> >>> -    if (!extra_size                                               ||
> >>> -        (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) ||
> >>> -        (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
> >>> +    if (is_annexb(ctx->par_in->extradata, extra_size)) {
> >>>        av_log(ctx, AV_LOG_VERBOSE,
> >>>               "The input looks like it is Annex B already\n");
> >>>    } else if (extra_size >= 7) {
> >>> @@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext
> >> *ctx, AVPacket *opkt)
> >>>    extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
> >>>                                        &extradata_size);
> >>>    if (extradata) {
> >>> -        ret = h264_extradata_to_annexb(ctx, extradata,
> extradata_size);
> >>> -        if (ret < 0)
> >>> -            goto fail;
> >>> +        if (!is_annexb(extradata, extradata_size) && extradata_size >=
> >> 7) {
> >>> +            ret = h264_extradata_to_annexb(ctx, extradata,
> >> extradata_size);
> >>> +            if (ret < 0)
> >>> +                goto fail;
> >>> +            av_packet_side_data_remove(in->side_data,
> >> &in->side_data_elems,
> >>> +                                       AV_PKT_DATA_NEW_EXTRADATA);
> >>> +            uint8_t *data = av_packet_new_side_data(
> >>> +                in, AV_PKT_DATA_NEW_EXTRADATA,
> >> ctx->par_out->extradata_size);
> >>> +            memcpy(data, ctx->par_out->extradata,
> >> ctx->par_out->extradata_size);
> >>> +        }
> >>
> >> Remove adding AV_PKT_DATA_NEW_EXTRADATA. There’re in-band PS now.
> >>
> >> There is another case which not belonging to this patch. With
> >> AV_PKT_DATA_NEW_EXTRADATA
> >> in annexb format, it should be inserted as in-band PS and also be
> removed
> >> after.
> >>
> >>>    }
> >>>
> >>>    /* nothing to filter */
> >>> diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
> >>> index 00b153cc93..7b3e865955 100644
> >>> --- a/tests/fate/h264.mak
> >>> +++ b/tests/fate/h264.mak
> >>> @@ -231,6 +231,7 @@ FATE_H264-$(call DEMMUX, MOV, H264,
> >> H264_MP4TOANNEXB_BSF) += fate-h264-bsf-mp4to
> >>>
> >> fate-h264-bsf-mp4toannexb-2 \
> >>>
> >> fate-h264_mp4toannexb_ticket5927 \
> >>>
> >> fate-h264_mp4toannexb_ticket5927_2 \
> >>> +
> >> fate-h264-bsf-mp4toannexb-h264_metadata \
> >>>
> >>> FATE_H264-$(call DEMMUX, H264, MOV, DTS2PTS_BSF) +=
> fate-h264-bsf-dts2pts
> >>>
> >>> @@ -441,6 +442,8 @@ fate-h264-bsf-mp4toannexb-2:
> >> CMD = md5 -i $(TARGET_SAMPLES)
> >>> fate-h264-bsf-mp4toannexb-2:                      CMP = oneline
> >>> fate-h264-bsf-mp4toannexb-2:                      REF =
> >> cffcfa6a2d0b58c9de1f5785f099f41d
> >>> fate-h264-bsf-mp4toannexb-new-extradata:          CMD = stream_remux
> mov
> >> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov "" h264 "-map
> 0:v"
> >>> +fate-h264-bsf-mp4toannexb-h264_metadata:          CMD = transcode mov
> >> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov \
> >>> +                                                        h264 "-c:v
> copy
> >> -bsf:v h264_mp4toannexb,h264_metadata=aud=remove" "-c:v copy"
> >>> fate-h264-bsf-dts2pts:                            CMD = transcode
> "h264"
> >> $(TARGET_SAMPLES)/h264-conformance/CAPAMA3_Sand_F.264 \
> >>>                                                        mov "-c:v copy
> >> -bsf:v dts2pts -frames:v 50" "-c:v copy"
> >>> fate-h264_mp4toannexb_ticket5927:                 CMD = transcode "mp4"
> >> $(TARGET_SAMPLES)/h264/thezerotheorem-cut.mp4 \
> >>> diff --git a/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> >> b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> >>> new file mode 100644
> >>> index 0000000000..d3ba7f1886
> >>> --- /dev/null
> >>> +++ b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata
> >>> @@ -0,0 +1,12 @@
> >>> +a48f175bf3458410d1dd1907682b92d3
> >> *tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
> >>> +4642 tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264
> >>> +#extradata 0:       38, 0x84d70914
> >>> +#tb 0: 1/1200000
> >>> +#media_type 0: video
> >>> +#codec_id 0: h264
> >>> +#dimensions 0: 256x128
> >>> +#sar 0: 1/1
> >>> +0,     -96000, -9223372036854775808,    48000,     2331, 0xc97509cb
> >>> +0,     -48000, -9223372036854775808,    48000,       64, 0x6ad91f4e,
> >> F=0x0
> >>> +0,          0, -9223372036854775808,    48000,     2203, 0xe376d2a9
> >>> +0,      48000, -9223372036854775808,    48000,       44, 0x687a10ea,
> >> F=0x0
> >>> --
> >>> 2.49.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".
> >>
> > _______________________________________________
> > 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 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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-04-25  2:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-24  3:38 [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc jiangjie
2025-04-24  4:19 ` Zhao Zhili
2025-04-24  5:18   ` jie jiang
2025-04-24  6:41     ` Zhao Zhili
2025-04-24 12:42 ` jiangjie
2025-04-24 13:04   ` jie jiang
2025-04-24 14:03     ` Zhao Zhili
2025-04-24 14:20   ` Zhao Zhili
2025-04-24 14:53     ` jie jiang
2025-04-24 17:23       ` Zhao Zhili
2025-04-25  2:41         ` jie jiang

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git