| Message ID | 20160926173913.976-2-jamrial@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Sep 26, 2016 at 02:39:13PM -0300, James Almer wrote: > This commit is based on commit 35c8580 from Anton Khirnov <anton@khirnov.net> > which was skipped in b8945c4. > > The avcodec_copy_context() call in the encode path is left in place for now > as AVStream.codec is apparently still required even after porting ffmpeg to > the new bsf API. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > ffmpeg.c | 25 ++++++++++++------------- > ffmpeg.h | 1 + > ffmpeg_opt.c | 12 +----------- > 3 files changed, 14 insertions(+), 24 deletions(-) This breaks ./ffmpeg -i matrixbench_mpeg2.mpg -flags +bitexact -t 1 -codec copy -f framecrc - compared to the case before the patch and without codec copy "#software: Lavf57.50.100" is output [...]
On 9/26/2016 6:38 PM, Michael Niedermayer wrote: > On Mon, Sep 26, 2016 at 02:39:13PM -0300, James Almer wrote: >> This commit is based on commit 35c8580 from Anton Khirnov <anton@khirnov.net> >> which was skipped in b8945c4. >> >> The avcodec_copy_context() call in the encode path is left in place for now >> as AVStream.codec is apparently still required even after porting ffmpeg to >> the new bsf API. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> ffmpeg.c | 25 ++++++++++++------------- >> ffmpeg.h | 1 + >> ffmpeg_opt.c | 12 +----------- >> 3 files changed, 14 insertions(+), 24 deletions(-) > > This breaks > ./ffmpeg -i matrixbench_mpeg2.mpg -flags +bitexact -t 1 -codec copy -f framecrc - > compared to the case before the patch and without codec copy > "#software: Lavf57.50.100" > is output AVCodecContext flag bitexact is (for now) converted into AVFormatContext flag bitexact, and by removing the chunk in ffmpeg_opt.c that sets the former in st->codec, this doesn't happen anymore. This is from init_muxer() libavformat/mux.c, and it's scheduled for removal with FF_API_LAVF_BITEXACT and ultimately with FF_API_LAVF_AVCTX, so i guess i'll leave the chunk in question in place and just wrap it up with a check for the latter define. Is that ok?
On Mon, Sep 26, 2016 at 07:17:20PM -0300, James Almer wrote: > On 9/26/2016 6:38 PM, Michael Niedermayer wrote: > > On Mon, Sep 26, 2016 at 02:39:13PM -0300, James Almer wrote: > >> This commit is based on commit 35c8580 from Anton Khirnov <anton@khirnov.net> > >> which was skipped in b8945c4. > >> > >> The avcodec_copy_context() call in the encode path is left in place for now > >> as AVStream.codec is apparently still required even after porting ffmpeg to > >> the new bsf API. > >> > >> Signed-off-by: James Almer <jamrial@gmail.com> > >> --- > >> ffmpeg.c | 25 ++++++++++++------------- > >> ffmpeg.h | 1 + > >> ffmpeg_opt.c | 12 +----------- > >> 3 files changed, 14 insertions(+), 24 deletions(-) > > > > This breaks > > ./ffmpeg -i matrixbench_mpeg2.mpg -flags +bitexact -t 1 -codec copy -f framecrc - > > compared to the case before the patch and without codec copy > > "#software: Lavf57.50.100" > > is output > > AVCodecContext flag bitexact is (for now) converted into AVFormatContext flag > bitexact, and by removing the chunk in ffmpeg_opt.c that sets the former in > st->codec, this doesn't happen anymore. > > This is from init_muxer() libavformat/mux.c, and it's scheduled for removal > with FF_API_LAVF_BITEXACT and ultimately with FF_API_LAVF_AVCTX, so i guess > i'll leave the chunk in question in place and just wrap it up with a check > for the latter define. Is that ok? ok, can you post the resulting patch so i can test if it breaks anything else ? also totally off topic but related we should add a '-bitexact' option instead of requiring to set both flags. i think someone had suggested this previously but it seems to be sliping down on my todo list ... if someone wants to add that ... [...]
diff --git a/ffmpeg.c b/ffmpeg.c index df55a49..bbde63b 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -516,6 +516,7 @@ static void ffmpeg_cleanup(int ret) av_dict_free(&ost->encoder_opts); av_parser_close(ost->parser); + avcodec_free_context(&ost->parser_avctx); av_freep(&ost->forced_keyframes); av_expr_free(ost->forced_keyframes_pexpr); @@ -1899,7 +1900,7 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p && ost->st->codecpar->codec_id != AV_CODEC_ID_MPEG2VIDEO && ost->st->codecpar->codec_id != AV_CODEC_ID_VC1 ) { - int ret = av_parser_change(ost->parser, ost->st->codec, + int ret = av_parser_change(ost->parser, ost->parser_avctx, &opkt.data, &opkt.size, pkt->data, pkt->size, pkt->flags & AV_PKT_FLAG_KEY); @@ -2723,9 +2724,7 @@ static int init_output_stream(OutputStream *ost, char *error, int error_len) exit_program(1); } /* - * FIXME: this is only so that the bitstream filters and parsers (that still - * work with a codec context) get the parameter values. - * This should go away with the new BSF/parser API. + * FIXME: ost->st->codec should't be needed here anymore. */ ret = avcodec_copy_context(ost->st->codec, ost->enc_ctx); if (ret < 0) @@ -2757,15 +2756,11 @@ static int init_output_stream(OutputStream *ost, char *error, int error_len) ost->st->time_base = av_add_q(ost->enc_ctx->time_base, (AVRational){0, 1}); ost->st->codec->codec= ost->enc_ctx->codec; } else if (ost->stream_copy) { - // copy timebase while removing common factors - ost->st->time_base = av_add_q(ost->st->codec->time_base, (AVRational){0, 1}); - /* - * FIXME: this is only so that the bitstream filters and parsers (that still - * work with a codec context) get the parameter values. - * This should go away with the new BSF/parser API. + * FIXME: will the codec context used by the parser during streamcopy + * This should go away with the new parser API. */ - ret = avcodec_parameters_to_context(ost->st->codec, ost->st->codecpar); + ret = avcodec_parameters_to_context(ost->parser_avctx, ost->st->codecpar); if (ret < 0) return ret; } @@ -3007,12 +3002,13 @@ static int transcode_init(void) ost->frame_rate = ist->framerate; ost->st->avg_frame_rate = ost->frame_rate; - ost->st->time_base = ist->st->time_base; - ret = avformat_transfer_internal_stream_timing_info(oc->oformat, ost->st, ist->st, copy_tb); if (ret < 0) return ret; + // copy timebase while removing common factors + ost->st->time_base = av_add_q(av_stream_get_codec_timebase(ost->st), (AVRational){0, 1}); + if (ist->st->nb_side_data) { ost->st->side_data = av_realloc_array(NULL, ist->st->nb_side_data, sizeof(*ist->st->side_data)); @@ -3038,6 +3034,9 @@ static int transcode_init(void) } ost->parser = av_parser_init(par_dst->codec_id); + ost->parser_avctx = avcodec_alloc_context3(NULL); + if (!ost->parser_avctx) + return AVERROR(ENOMEM); switch (par_dst->codec_type) { case AVMEDIA_TYPE_AUDIO: diff --git a/ffmpeg.h b/ffmpeg.h index b7f8b7a..0d01d2b 100644 --- a/ffmpeg.h +++ b/ffmpeg.h @@ -477,6 +477,7 @@ typedef struct OutputStream { int keep_pix_fmt; AVCodecParserContext *parser; + AVCodecContext *parser_avctx; /* stats */ // combined size of all the packets written diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c index 73da546..0dd06a7 100644 --- a/ffmpeg_opt.c +++ b/ffmpeg_opt.c @@ -2300,6 +2300,7 @@ loop_end: avio_read(pb, attachment, len); ost = new_attachment_stream(o, oc, -1); + ost->stream_copy = 0; ost->attachment_filename = o->attachments[i]; ost->st->codecpar->extradata = attachment; ost->st->codecpar->extradata_size = len; @@ -2309,17 +2310,6 @@ loop_end: avio_closep(&pb); } - for (i = nb_output_streams - oc->nb_streams; i < nb_output_streams; i++) { //for all streams of this output file - AVDictionaryEntry *e; - ost = output_streams[i]; - - if ((ost->stream_copy || ost->attachment_filename) - && (e = av_dict_get(o->g->codec_opts, "flags", NULL, AV_DICT_IGNORE_SUFFIX)) - && (!e->key[5] || check_stream_specifier(oc, ost->st, e->key+6))) - if (av_opt_set(ost->st->codec, "flags", e->value, 0) < 0) - exit_program(1); - } - if (!oc->nb_streams && !(oc->oformat->flags & AVFMT_NOSTREAMS)) { av_dump_format(oc, nb_output_files - 1, oc->filename, 1); av_log(NULL, AV_LOG_ERROR, "Output file #%d does not contain any stream\n", nb_output_files - 1);
This commit is based on commit 35c8580 from Anton Khirnov <anton@khirnov.net> which was skipped in b8945c4. The avcodec_copy_context() call in the encode path is left in place for now as AVStream.codec is apparently still required even after porting ffmpeg to the new bsf API. Signed-off-by: James Almer <jamrial@gmail.com> --- ffmpeg.c | 25 ++++++++++++------------- ffmpeg.h | 1 + ffmpeg_opt.c | 12 +----------- 3 files changed, 14 insertions(+), 24 deletions(-)