| Message ID | 20200510211947.3247-2-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | 2932905255995cd615fe01126e233e26112b9e5e |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/ivfenc: add an AVCodec.init() function | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
James Almer: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/ivfenc.c | 24 ++++++++++-------------- > 1 file changed, 10 insertions(+), 14 deletions(-) > > diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c > index 0ce4a85171..0951f56c92 100644 > --- a/libavformat/ivfenc.c > +++ b/libavformat/ivfenc.c > @@ -43,6 +43,16 @@ static int ivf_init(AVFormatContext *s) > return AVERROR(EINVAL); > } > > + if (par->codec_id == AV_CODEC_ID_VP9) { > + int ret = ff_stream_add_bitstream_filter(s->streams[0], "vp9_superframe", NULL); > + if (ret < 0) > + return ret; > + } else if (par->codec_id == AV_CODEC_ID_AV1) { > + int ret = ff_stream_add_bitstream_filter(s->streams[0], "av1_metadata", "td=insert"); > + if (ret < 0) > + return ret; > + } > + > return 0; > } > > @@ -100,19 +110,6 @@ static int ivf_write_trailer(AVFormatContext *s) > return 0; > } > > -static int ivf_check_bitstream(struct AVFormatContext *s, const AVPacket *pkt) > -{ > - int ret = 1; > - AVStream *st = s->streams[pkt->stream_index]; > - > - if (st->codecpar->codec_id == AV_CODEC_ID_VP9) > - ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); > - else if (st->codecpar->codec_id == AV_CODEC_ID_AV1) > - ret = ff_stream_add_bitstream_filter(st, "av1_metadata", "td=insert"); > - > - return ret; > -} > - > static const AVCodecTag codec_ivf_tags[] = { > { AV_CODEC_ID_VP8, MKTAG('V', 'P', '8', '0') }, > { AV_CODEC_ID_VP9, MKTAG('V', 'P', '9', '0') }, > @@ -131,6 +128,5 @@ AVOutputFormat ff_ivf_muxer = { > .write_header = ivf_write_header, > .write_packet = ivf_write_packet, > .write_trailer = ivf_write_trailer, > - .check_bitstream = ivf_check_bitstream, > .codec_tag = (const AVCodecTag* const []){ codec_ivf_tags, 0 }, > }; > LGTM. (Would it actually make sense/be possible to replace st->codecpar with the output codecparameters of the bsf if the bsf is initialized in the init function, so that it is easier for the muxer to locate the right codecparameters when writing the header?) - Andreas
Quoting Andreas Rheinhardt (2020-05-11 13:59:27) > James Almer: > > Signed-off-by: James Almer <jamrial@gmail.com> > > --- > > libavformat/ivfenc.c | 24 ++++++++++-------------- > > 1 file changed, 10 insertions(+), 14 deletions(-) > > > > diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c > > index 0ce4a85171..0951f56c92 100644 > > --- a/libavformat/ivfenc.c > > +++ b/libavformat/ivfenc.c > > @@ -43,6 +43,16 @@ static int ivf_init(AVFormatContext *s) > > return AVERROR(EINVAL); > > } > > > > + if (par->codec_id == AV_CODEC_ID_VP9) { > > + int ret = ff_stream_add_bitstream_filter(s->streams[0], "vp9_superframe", NULL); > > + if (ret < 0) > > + return ret; > > + } else if (par->codec_id == AV_CODEC_ID_AV1) { > > + int ret = ff_stream_add_bitstream_filter(s->streams[0], "av1_metadata", "td=insert"); > > + if (ret < 0) > > + return ret; > > + } > > + > > return 0; > > } > > > > @@ -100,19 +110,6 @@ static int ivf_write_trailer(AVFormatContext *s) > > return 0; > > } > > > > -static int ivf_check_bitstream(struct AVFormatContext *s, const AVPacket *pkt) > > -{ > > - int ret = 1; > > - AVStream *st = s->streams[pkt->stream_index]; > > - > > - if (st->codecpar->codec_id == AV_CODEC_ID_VP9) > > - ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); > > - else if (st->codecpar->codec_id == AV_CODEC_ID_AV1) > > - ret = ff_stream_add_bitstream_filter(st, "av1_metadata", "td=insert"); > > - > > - return ret; > > -} > > - > > static const AVCodecTag codec_ivf_tags[] = { > > { AV_CODEC_ID_VP8, MKTAG('V', 'P', '8', '0') }, > > { AV_CODEC_ID_VP9, MKTAG('V', 'P', '9', '0') }, > > @@ -131,6 +128,5 @@ AVOutputFormat ff_ivf_muxer = { > > .write_header = ivf_write_header, > > .write_packet = ivf_write_packet, > > .write_trailer = ivf_write_trailer, > > - .check_bitstream = ivf_check_bitstream, > > .codec_tag = (const AVCodecTag* const []){ codec_ivf_tags, 0 }, > > }; > > > LGTM. (Would it actually make sense/be possible to replace st->codecpar > with the output codecparameters of the bsf if the bsf is initialized in > the init function, so that it is easier for the muxer to locate the > right codecparameters when writing the header?) I'd say ideally the muxer internals should be decoupled from the outwards user-facing AVFormatContext/AVStream and not touch them directly. Same for the codecs. It would probably take a lot of work to implement though.
On 5/11/2020 8:59 AM, Andreas Rheinhardt wrote: > James Almer: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/ivfenc.c | 24 ++++++++++-------------- >> 1 file changed, 10 insertions(+), 14 deletions(-) >> >> diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c >> index 0ce4a85171..0951f56c92 100644 >> --- a/libavformat/ivfenc.c >> +++ b/libavformat/ivfenc.c >> @@ -43,6 +43,16 @@ static int ivf_init(AVFormatContext *s) >> return AVERROR(EINVAL); >> } >> >> + if (par->codec_id == AV_CODEC_ID_VP9) { >> + int ret = ff_stream_add_bitstream_filter(s->streams[0], "vp9_superframe", NULL); >> + if (ret < 0) >> + return ret; >> + } else if (par->codec_id == AV_CODEC_ID_AV1) { >> + int ret = ff_stream_add_bitstream_filter(s->streams[0], "av1_metadata", "td=insert"); >> + if (ret < 0) >> + return ret; >> + } >> + >> return 0; >> } >> >> @@ -100,19 +110,6 @@ static int ivf_write_trailer(AVFormatContext *s) >> return 0; >> } >> >> -static int ivf_check_bitstream(struct AVFormatContext *s, const AVPacket *pkt) >> -{ >> - int ret = 1; >> - AVStream *st = s->streams[pkt->stream_index]; >> - >> - if (st->codecpar->codec_id == AV_CODEC_ID_VP9) >> - ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); >> - else if (st->codecpar->codec_id == AV_CODEC_ID_AV1) >> - ret = ff_stream_add_bitstream_filter(st, "av1_metadata", "td=insert"); >> - >> - return ret; >> -} >> - >> static const AVCodecTag codec_ivf_tags[] = { >> { AV_CODEC_ID_VP8, MKTAG('V', 'P', '8', '0') }, >> { AV_CODEC_ID_VP9, MKTAG('V', 'P', '9', '0') }, >> @@ -131,6 +128,5 @@ AVOutputFormat ff_ivf_muxer = { >> .write_header = ivf_write_header, >> .write_packet = ivf_write_packet, >> .write_trailer = ivf_write_trailer, >> - .check_bitstream = ivf_check_bitstream, >> .codec_tag = (const AVCodecTag* const []){ codec_ivf_tags, 0 }, >> }; >> > LGTM. (Would it actually make sense/be possible to replace st->codecpar > with the output codecparameters of the bsf if the bsf is initialized in > the init function, so that it is easier for the muxer to locate the > right codecparameters when writing the header?) It's possible, but i don't know if that's expected behavior or not. Editing the codecpar is done by some muxers, true, but the doxy stats it's "filled by the caller before avformat_write_header()". An option could be to have a copy of the AVCodecParameters that can be edited in st->internal instead, and leave the caller one in st alone. Will apply, thanks.
diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c index 0ce4a85171..0951f56c92 100644 --- a/libavformat/ivfenc.c +++ b/libavformat/ivfenc.c @@ -43,6 +43,16 @@ static int ivf_init(AVFormatContext *s) return AVERROR(EINVAL); } + if (par->codec_id == AV_CODEC_ID_VP9) { + int ret = ff_stream_add_bitstream_filter(s->streams[0], "vp9_superframe", NULL); + if (ret < 0) + return ret; + } else if (par->codec_id == AV_CODEC_ID_AV1) { + int ret = ff_stream_add_bitstream_filter(s->streams[0], "av1_metadata", "td=insert"); + if (ret < 0) + return ret; + } + return 0; } @@ -100,19 +110,6 @@ static int ivf_write_trailer(AVFormatContext *s) return 0; } -static int ivf_check_bitstream(struct AVFormatContext *s, const AVPacket *pkt) -{ - int ret = 1; - AVStream *st = s->streams[pkt->stream_index]; - - if (st->codecpar->codec_id == AV_CODEC_ID_VP9) - ret = ff_stream_add_bitstream_filter(st, "vp9_superframe", NULL); - else if (st->codecpar->codec_id == AV_CODEC_ID_AV1) - ret = ff_stream_add_bitstream_filter(st, "av1_metadata", "td=insert"); - - return ret; -} - static const AVCodecTag codec_ivf_tags[] = { { AV_CODEC_ID_VP8, MKTAG('V', 'P', '8', '0') }, { AV_CODEC_ID_VP9, MKTAG('V', 'P', '9', '0') }, @@ -131,6 +128,5 @@ AVOutputFormat ff_ivf_muxer = { .write_header = ivf_write_header, .write_packet = ivf_write_packet, .write_trailer = ivf_write_trailer, - .check_bitstream = ivf_check_bitstream, .codec_tag = (const AVCodecTag* const []){ codec_ivf_tags, 0 }, };
Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/ivfenc.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-)