| Message ID | 20170426194055.5672-1-jamrial@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Apr 26, 2017 at 04:40:55PM -0300, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/concatdec.c | 86 +++++++++++++++---------------------------------- > 1 file changed, 26 insertions(+), 60 deletions(-) breaks ./ffmpeg -f concat -i ~/tickets/3108/concatfile.txt -codec copy test.avi (output produces many warnings/errors on playback) https://trac.ffmpeg.org/raw-attachment/ticket/3108/examplefiles.zip [...]
On 4/27/2017 10:59 PM, Michael Niedermayer wrote: > On Wed, Apr 26, 2017 at 04:40:55PM -0300, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/concatdec.c | 86 +++++++++++++++---------------------------------- >> 1 file changed, 26 insertions(+), 60 deletions(-) > > breaks > ./ffmpeg -f concat -i ~/tickets/3108/concatfile.txt -codec copy test.avi > (output produces many warnings/errors on playback) > https://trac.ffmpeg.org/raw-attachment/ticket/3108/examplefiles.zip Huh, this was more broken than i thought. Apparently, concatdec was never really filtering anything before this patch. detect_stream_specific() frees up the stream's extradata before the code ever has the chance to call the bsf init function because, despite the name, the compat code in av_bitstream_filter_init() does not call it. That only happens in the first av_bitstream_filter_filter() call. The h264_mp4toannexb bsf looks at extradata during init to figure out if it needs to filter anything or just do a packet passthrough. That aside, concatdec was also copying the input file's stream extradata to the matched output stream extradata *before* it called detect_stream_specific(), so any filtered extradata would have been ignored anyway. I don't know if this started happening after the new bsf API was introduced and the old converted into a wrapper for the new, or if it was always like this, but after this conversion it will not matter. Will send an updated patch in a moment.
On 4/28/2017 9:16 PM, James Almer wrote: > On 4/27/2017 10:59 PM, Michael Niedermayer wrote: >> On Wed, Apr 26, 2017 at 04:40:55PM -0300, James Almer wrote: >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavformat/concatdec.c | 86 +++++++++++++++---------------------------------- >>> 1 file changed, 26 insertions(+), 60 deletions(-) >> >> breaks >> ./ffmpeg -f concat -i ~/tickets/3108/concatfile.txt -codec copy test.avi >> (output produces many warnings/errors on playback) >> https://trac.ffmpeg.org/raw-attachment/ticket/3108/examplefiles.zip > > Huh, this was more broken than i thought. Apparently, concatdec was > never really filtering anything before this patch. > > detect_stream_specific() frees up the stream's extradata before the code > ever has the chance to call the bsf init function because, despite the > name, the compat code in av_bitstream_filter_init() does not call it. > That only happens in the first av_bitstream_filter_filter() call. The > h264_mp4toannexb bsf looks at extradata during init to figure out if it > needs to filter anything or just do a packet passthrough. > That aside, concatdec was also copying the input file's stream extradata > to the matched output stream extradata *before* it called > detect_stream_specific(), so any filtered extradata would have been > ignored anyway. > > I don't know if this started happening after the new bsf API was > introduced and the old converted into a wrapper for the new, or if it > was always like this, but after this conversion it will not matter. Commit 0cb19c30c6 essentially disabled the bsf. The "fix" it mentions is because matroska stores avcc instead of annexb, so it just muxed the stream untouched. This means actual filtering was broken beforehand, probably by b8fa374fb6 (Letting unfiltered extradata make it to the output file's matched stream while the actual h264 stream was filtered).
diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c index dd52e4d366..97ef41e3fc 100644 --- a/libavformat/concatdec.c +++ b/libavformat/concatdec.c @@ -34,8 +34,7 @@ typedef enum ConcatMatchMode { } ConcatMatchMode; typedef struct ConcatStream { - AVBitStreamFilterContext *bsf; - AVCodecContext *avctx; + AVBSFContext *bsf; int out_stream_index; } ConcatStream; @@ -196,7 +195,8 @@ static int detect_stream_specific(AVFormatContext *avf, int idx) ConcatContext *cat = avf->priv_data; AVStream *st = cat->avf->streams[idx]; ConcatStream *cs = &cat->cur_file->streams[idx]; - AVBitStreamFilterContext *bsf; + const AVBitStreamFilter *filter; + AVBSFContext *bsf; int ret; if (cat->auto_convert && st->codecpar->codec_id == AV_CODEC_ID_H264) { @@ -206,29 +206,28 @@ static int detect_stream_specific(AVFormatContext *avf, int idx) return 0; av_log(cat->avf, AV_LOG_INFO, "Auto-inserting h264_mp4toannexb bitstream filter\n"); - if (!(bsf = av_bitstream_filter_init("h264_mp4toannexb"))) { + filter = av_bsf_get_by_name("h264_mp4toannexb"); + if (!filter) { av_log(avf, AV_LOG_ERROR, "h264_mp4toannexb bitstream filter " "required for H.264 streams\n"); return AVERROR_BSF_NOT_FOUND; } + ret = av_bsf_alloc(filter, &bsf); + if (ret < 0) + return ret; cs->bsf = bsf; - cs->avctx = avcodec_alloc_context3(NULL); - if (!cs->avctx) - return AVERROR(ENOMEM); - - /* This really should be part of the bsf work. - Note: input bitstream filtering will not work with bsf that - create extradata from the first packet. */ - av_freep(&st->codecpar->extradata); - st->codecpar->extradata_size = 0; + ret = avcodec_parameters_copy(bsf->par_in, st->codecpar); + if (ret < 0) + return ret; - ret = avcodec_parameters_to_context(cs->avctx, st->codecpar); - if (ret < 0) { - avcodec_free_context(&cs->avctx); + ret = av_bsf_init(bsf); + if (ret < 0) return ret; - } + ret = avcodec_parameters_copy(st->codecpar, bsf->par_out); + if (ret < 0) + return ret; } return 0; } @@ -370,10 +369,8 @@ static int concat_read_close(AVFormatContext *avf) for (i = 0; i < cat->nb_files; i++) { av_freep(&cat->files[i].url); for (j = 0; j < cat->files[i].nb_streams; j++) { - if (cat->files[i].streams[j].avctx) - avcodec_free_context(&cat->files[i].streams[j].avctx); if (cat->files[i].streams[j].bsf) - av_bitstream_filter_close(cat->files[i].streams[j].bsf); + av_bsf_free(&cat->files[i].streams[j].bsf); } av_freep(&cat->files[i].streams); av_dict_free(&cat->files[i].metadata); @@ -524,56 +521,25 @@ static int open_next_file(AVFormatContext *avf) static int filter_packet(AVFormatContext *avf, ConcatStream *cs, AVPacket *pkt) { - AVStream *st = avf->streams[cs->out_stream_index]; - AVBitStreamFilterContext *bsf; - AVPacket pkt2; int ret; av_assert0(cs->out_stream_index >= 0); - for (bsf = cs->bsf; bsf; bsf = bsf->next) { - pkt2 = *pkt; - - ret = av_bitstream_filter_filter(bsf, cs->avctx, NULL, - &pkt2.data, &pkt2.size, - pkt->data, pkt->size, - !!(pkt->flags & AV_PKT_FLAG_KEY)); + if (cs->bsf) { + ret = av_bsf_send_packet(cs->bsf, pkt); if (ret < 0) { + av_log(avf, AV_LOG_ERROR, "h264_mp4toannexb filter " + "failed to send input packet\n"); av_packet_unref(pkt); return ret; } - if (cs->avctx->extradata_size > st->codecpar->extradata_size) { - int eret; - if (st->codecpar->extradata) - av_freep(&st->codecpar->extradata); - - eret = ff_alloc_extradata(st->codecpar, cs->avctx->extradata_size); - if (eret < 0) { - av_packet_unref(pkt); - return AVERROR(ENOMEM); - } - st->codecpar->extradata_size = cs->avctx->extradata_size; - memcpy(st->codecpar->extradata, cs->avctx->extradata, cs->avctx->extradata_size); - } - - av_assert0(pkt2.buf); - if (ret == 0 && pkt2.data != pkt->data) { - if ((ret = av_copy_packet(&pkt2, pkt)) < 0) { - av_free(pkt2.data); - return ret; - } - ret = 1; - } - if (ret > 0) { + ret = av_bsf_receive_packet(cs->bsf, pkt); + if (ret < 0) { + av_log(avf, AV_LOG_ERROR, "h264_mp4toannexb filter " + "failed to receive output packet\n"); av_packet_unref(pkt); - pkt2.buf = av_buffer_create(pkt2.data, pkt2.size, - av_buffer_default_free, NULL, 0); - if (!pkt2.buf) { - av_free(pkt2.data); - return AVERROR(ENOMEM); - } + return ret; } - *pkt = pkt2; } return 0; }
Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/concatdec.c | 86 +++++++++++++++---------------------------------- 1 file changed, 26 insertions(+), 60 deletions(-)