Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: jie jiang <jiangjie618@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
Date: Thu, 24 Apr 2025 22:53:50 +0800
Message-ID: <CAJ53Rrti+equtw75xmz7q1AfodDPuin_Eb4=JwbnhWh6TQdWDQ@mail.gmail.com> (raw)
In-Reply-To: <tencent_74AB044E6570772D549AF0401F9A249FD608@qq.com>

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

  reply	other threads:[~2025-04-24 14:54 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-24  3:38 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 [this message]
2025-04-24 17:23       ` Zhao Zhili
2025-04-25  2:41         ` jie jiang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJ53Rrti+equtw75xmz7q1AfodDPuin_Eb4=JwbnhWh6TQdWDQ@mail.gmail.com' \
    --to=jiangjie618@gmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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