> On Apr 27, 2016, at 07:29, Nicolas George <[email protected]> wrote: > > Le nonidi 9 floréal, an CCXXIV, Rodger Combs a écrit : >> --- >> libavformat/internal.h | 5 +++-- >> libavformat/mux.c | 40 +++++++++++++++++++++++++--------- >> libavformat/segment.c | 6 +++-- >> libavformat/utils.c | 59 >> +++++++++++++++++++++++++++++++++++++++++--------- >> 4 files changed, 86 insertions(+), 24 deletions(-) >> >> diff --git a/libavformat/internal.h b/libavformat/internal.h >> index 40ba089..52f9eb6 100644 >> --- a/libavformat/internal.h >> +++ b/libavformat/internal.h >> @@ -134,11 +134,12 @@ struct AVStreamInternal { >> int reorder; >> >> /** >> - * bitstream filter to run on stream >> + * bitstream filters to run on stream >> * - encoding: Set by muxer using ff_stream_add_bitstream_filter >> * - decoding: unused >> */ >> - AVBitStreamFilterContext *bsfc; >> + AVBSFContext **bsfcs; >> + int nb_bsfcs; >> >> /** >> * Whether or not check_bitstream should still be run on each packet >> diff --git a/libavformat/mux.c b/libavformat/mux.c >> index 91388e3..72e284a 100644 >> --- a/libavformat/mux.c >> +++ b/libavformat/mux.c >> @@ -1043,7 +1043,7 @@ static int interleave_packet(AVFormatContext *s, >> AVPacket *out, AVPacket *in, in >> >> int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt) >> { >> - int ret, flush = 0; >> + int ret, flush = 0, i; >> >> ret = prepare_input_packet(s, pkt); >> if (ret < 0) >> @@ -1061,15 +1061,35 @@ int av_interleaved_write_frame(AVFormatContext *s, >> AVPacket *pkt) >> } >> } >> >> - av_apply_bitstream_filters(st->internal->avctx, pkt, >> st->internal->bsfc); >> - if (pkt->size == 0 && pkt->side_data_elems == 0) >> - return 0; >> - if (!st->codecpar->extradata && st->internal->avctx->extradata) { >> - int eret = ff_alloc_extradata(st->codecpar, >> st->internal->avctx->extradata_size); >> - if (eret < 0) >> - return AVERROR(ENOMEM); >> - st->codecpar->extradata_size = >> st->internal->avctx->extradata_size; >> - memcpy(st->codecpar->extradata, st->internal->avctx->extradata, >> st->internal->avctx->extradata_size); > >> + for (i = 0; i < st->internal->nb_bsfcs; i++) { >> + AVBSFContext* ctx = st->internal->bsfcs[i]; > > Style nit: the pointer mark is part of the variable, not the type. Fixed.
>
> And: Unless I am mistaken, for now, we only ever add a single bitstream
> filter. The new API is tricky with several filters. I suggest you write the
> code for a single filter and leave implementing several until we actually
> need it, and therefore have a test case for it.
There's definitely going to be some trickiness around this if we end up having
1:N filters, or filters that actually requiring flushing. Otherwise, though,
this should work fine, and we can address multiple-bsf or more-advanced BSF
cases when they come up.
>
>> + if (i > 0) {
>> + AVBSFContext* prev_ctx = st->internal->bsfcs[i - 1];
>> + if (prev_ctx->par_out->extradata_size !=
>> ctx->par_in->extradata_size) {
>> + if ((ret = avcodec_parameters_copy(ctx->par_in,
>> prev_ctx->par_out)) < 0)
>> + goto fail;
>> + }
>> + }
>
>> + if ((ret = av_bsf_send_packet(ctx, pkt)) < 0) {
>
> I notice this is the only use of this function. Can pkg be NULL? Otherwise,
> flushing the filter is missing.
pkt can't be NULL here. Flushing the filters will be somewhat complex
(requiring a loop over all the streams to flush them all), and AFAIK none of
the current automatically-added BSFs require it, so I'm punting on that for now.
>
>> + av_log(ctx, AV_LOG_ERROR,
>> + "Failed to send packet to filter %s for stream %d",
>> + ctx->filter->name, pkt->stream_index);
>> + goto fail;
>> + }
>
>> + if ((ret = av_bsf_receive_packet(ctx, pkt)) < 0) {
>> + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF)
>> + return 0;
>> + av_log(ctx, AV_LOG_ERROR,
>> + "Failed to send packet to filter %s for stream %d",
>> + ctx->filter->name, pkt->stream_index);
>> + goto fail;
>> + }
>
> This is not correct, there must be a loop because a single send can cause
> several receive.
This also doesn't happen with any current BSFs, and the required looping is
non-trivial, so I'm punting on this as well. Adding TODO comments for both
issues.
>
>> + if (i == st->internal->nb_bsfcs - 1) {
>> + if (ctx->par_out->extradata_size !=
>> st->codecpar->extradata_size) {
>> + if ((ret = avcodec_parameters_copy(st->codecpar,
>> ctx->par_out)) < 0)
>> + goto fail;
>> + }
>> + }
>> }
>>
>> if (s->debug & FF_FDEBUG_TS)
>
> Regards,
>
> --
> Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> [email protected] <mailto:[email protected]>
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ ffmpeg-devel mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
