[FFmpeg-devel] [PATCH] avformat/tee: Move to new BSF API
Nicolas George
george at nsup.org
Wed May 11 21:48:04 CEST 2016
Le tridi 23 floréal, an CCXXIV, sebechlebskyjan at gmail.com a écrit :
> From: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
>
> Signed-off-by: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
> ---
> libavformat/tee.c | 171 ++++++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 139 insertions(+), 32 deletions(-)
Just a few comments for the first round.
>
> diff --git a/libavformat/tee.c b/libavformat/tee.c
> index 806beaa..ff0918b 100644
> --- a/libavformat/tee.c
> +++ b/libavformat/tee.c
> @@ -36,9 +36,14 @@ typedef enum {
>
> #define DEFAULT_SLAVE_FAILURE_POLICY ON_SLAVE_FAILURE_ABORT
>
> +typedef struct TeeBSFList {
> + AVBSFContext *bsf_ctx;
> + struct TeeBSFList *next;
> +} TeeBSFList;
The code use more arrays than linked lists usually.
> +
> typedef struct {
> AVFormatContext *avf;
> - AVBitStreamFilterContext **bsfs; ///< bitstream filters per stream
> + TeeBSFList **bsfs; ///< bitstream filters per stream
>
> SlaveFailurePolicy on_fail;
>
> @@ -113,30 +118,61 @@ fail:
> * The list must be specified in the form:
> * BSFS ::= BSF[,BSFS]
> */
> -static int parse_bsfs(void *log_ctx, const char *bsfs_spec,
> - AVBitStreamFilterContext **bsfs)
> +static int parse_bsfs(AVFormatContext *avf, const char *bsfs_spec,
> + TeeBSFList **bsfs, int stream_nr)
> {
> char *bsf_name, *buf, *dup, *saveptr;
> int ret = 0;
> + const AVBitStreamFilter *filter;
> + AVBSFContext *bsf_ctx;
> + TeeBSFList *bsf_lst;
> + AVStream *stream = avf->streams[stream_nr];
> + AVRational last_tb = stream->time_base;
>
> if (!(dup = buf = av_strdup(bsfs_spec)))
> return AVERROR(ENOMEM);
>
> while (bsf_name = av_strtok(buf, ",", &saveptr)) {
> - AVBitStreamFilterContext *bsf = av_bitstream_filter_init(bsf_name);
> + filter = av_bsf_get_by_name(bsf_name);
> +
> + if (!filter) {
> + av_log(avf, AV_LOG_ERROR, "Unknown bitstream filter '%s'\n",
> + bsf_name);
> + ret = AVERROR(EINVAL);
> + goto end;
> + }
>
> - if (!bsf) {
> - av_log(log_ctx, AV_LOG_ERROR,
> - "Cannot initialize bitstream filter with name '%s', "
> - "unknown filter or internal error happened\n",
> + if ((ret = av_bsf_alloc(filter, &bsf_ctx)) < 0) {
> + av_log(avf, AV_LOG_ERROR, "Cannot initialize bitstream filter '%s'",
> bsf_name);
> - ret = AVERROR_UNKNOWN;
> goto end;
> }
>
> - /* append bsf context to the list of bsf contexts */
> - *bsfs = bsf;
> - bsfs = &bsf->next;
> + ret = avcodec_parameters_copy(bsf_ctx->par_in, stream->codecpar);
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + bsf_ctx->time_base_in = last_tb;
> +
> + if ((ret = av_bsf_init(bsf_ctx)) < 0) {
> + goto fail;
> + }
> +
> + last_tb = bsf_ctx->time_base_out;
> +
> + /* allocate new bsf list node and append to the list of bsf contexts */
> + bsf_lst = av_mallocz(sizeof(TeeBSFList));
> +
> + if (!bsf_lst) {
> + ret = AVERROR(ENOMEM);
> + goto fail;
> + }
> +
> + bsf_lst->bsf_ctx = bsf_ctx;
> +
> + *bsfs = bsf_lst;
> + bsfs = &bsf_lst->next;
>
> buf = NULL;
> }
> @@ -144,6 +180,10 @@ static int parse_bsfs(void *log_ctx, const char *bsfs_spec,
> end:
> av_free(dup);
> return ret;
> +fail:
> + av_free(dup);
> + av_bsf_free(&bsf_ctx);
> + return ret;
> }
>
> static inline int parse_slave_failure_policy_option(const char *opt, TeeSlave *tee_slave)
> @@ -163,6 +203,75 @@ static inline int parse_slave_failure_policy_option(const char *opt, TeeSlave *t
> return AVERROR(EINVAL);
> }
>
> +/**
> + * Apply bitstream filters and write frame(s)
> + */
> +static int tee_process_packet(AVFormatContext *avf, TeeBSFList *bsf, AVPacket *pkt,
> + AVRational pkt_tb, int stream_nr)
> +{
> + int ret_all = 0,ret;
> + if (!bsf) {
> + if (pkt) {
> + AVRational out_tb = avf->streams[stream_nr]->time_base;
> + pkt->pts = av_rescale_q(pkt->pts, pkt_tb, out_tb);
> + pkt->dts = av_rescale_q(pkt->dts, pkt_tb, out_tb);
> + pkt->duration = av_rescale_q(pkt->duration, pkt_tb, out_tb);
> + pkt->stream_index = stream_nr;
> +
> + ret_all = av_interleaved_write_frame(avf, pkt);
> + }
Maybe this could benefit from the null bsf that I need to push soon.
> + goto end;
> + }
> +
> + if ((ret_all = av_bsf_send_packet(bsf->bsf_ctx, pkt)) < 0) {
> + return ret_all;
> + }
> +
> + do {
> + if (!(ret = av_bsf_receive_packet(bsf->bsf_ctx, pkt))) {
> + ret = tee_process_packet(avf, bsf->next, pkt,
> + bsf->bsf_ctx->time_base_out, stream_nr);
> + }
> + } while(!ret);
I am not really happy with the idea of recursive filtering coming back by
this path.
> +
> + if (ret != AVERROR(EAGAIN) && ret != AVERROR_EOF) {
> + ret_all = ret;
> + }
> +
> +end:
> + return ret_all;
> +}
> +
> +static void free_bsfs(TeeBSFList *bsf_list) {
> + TeeBSFList *next, *current = bsf_list;
> +
> + while (current) {
> + av_bsf_free(¤t->bsf_ctx);
> + next = current->next;
> + av_free(current);
> + current = next;
> + }
> +}
> +
> +static int flush_bsfs(TeeSlave *tee_slave)
> +{
> + AVFormatContext *avf = tee_slave->avf;
> + int i;
> + int ret, retAll = 0;
Please no camelCase for variable names.
> +
> + for (i = 0; i < avf->nb_streams; i++) {
> + if (tee_slave->bsfs) {
> + ret = tee_process_packet(avf, tee_slave->bsfs[i], NULL,
> + av_make_q(1,0), i);
> + if (!retAll && ret < 0) {
> + retAll = ret;
> + }
> + }
> + }
> +
> + return retAll;
> +}
> +
> static int close_slave(TeeSlave *tee_slave)
> {
> AVFormatContext *avf;
> @@ -173,17 +282,20 @@ static int close_slave(TeeSlave *tee_slave)
> if (!avf)
> return 0;
>
> - if (tee_slave->header_written)
> + if (tee_slave->header_written) {
> + ret = flush_bsfs(tee_slave);
> + if (ret < 0) {
> + av_log(avf, AV_LOG_ERROR, "Error flushing bitstream filters: %s\n",
> + av_err2str(ret));
> + }
> ret = av_write_trailer(avf);
> + }
> +
> + flush_bsfs(tee_slave);
>
> if (tee_slave->bsfs) {
> for (i = 0; i < avf->nb_streams; ++i) {
> - AVBitStreamFilterContext *bsf_next, *bsf = tee_slave->bsfs[i];
> - while (bsf) {
> - bsf_next = bsf->next;
> - av_bitstream_filter_close(bsf);
> - bsf = bsf_next;
> - }
> + free_bsfs(tee_slave->bsfs[i]);
> }
> }
> av_freep(&tee_slave->stream_map);
> @@ -362,7 +474,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> "output '%s', filters will be ignored\n", i, filename);
> continue;
> }
> - ret = parse_bsfs(avf, entry->value, &tee_slave->bsfs[i]);
> + ret = parse_bsfs(avf, entry->value, &tee_slave->bsfs[i], i);
> if (ret < 0) {
> av_log(avf, AV_LOG_ERROR,
> "Error parsing bitstream filter sequence '%s' associated to "
> @@ -399,7 +511,7 @@ static void log_slave(TeeSlave *slave, void *log_ctx, int log_level)
> slave->avf->filename, slave->avf->oformat->name);
> for (i = 0; i < slave->avf->nb_streams; i++) {
> AVStream *st = slave->avf->streams[i];
> - AVBitStreamFilterContext *bsf = slave->bsfs[i];
> + TeeBSFList *bsf = slave->bsfs[i];
>
> av_log(log_ctx, log_level, " stream:%d codec:%s type:%s",
> i, avcodec_get_name(st->codecpar->codec_id),
> @@ -408,7 +520,7 @@ static void log_slave(TeeSlave *slave, void *log_ctx, int log_level)
> av_log(log_ctx, log_level, " bsfs:");
> while (bsf) {
> av_log(log_ctx, log_level, "%s%s",
> - bsf->filter->name, bsf->next ? "," : "");
> + bsf->bsf_ctx->filter->name, bsf->next ? "," : "");
> bsf = bsf->next;
> }
> }
> @@ -516,7 +628,7 @@ static int tee_write_packet(AVFormatContext *avf, AVPacket *pkt)
> int ret_all = 0, ret;
> unsigned i, s;
> int s2;
> - AVRational tb, tb2;
> + AVRational tb;
>
> for (i = 0; i < tee->nb_slaves; i++) {
> if (!(avf2 = tee->slaves[i].avf))
> @@ -533,16 +645,11 @@ static int tee_write_packet(AVFormatContext *avf, AVPacket *pkt)
> ret_all = ret;
> continue;
> }
> +
> tb = avf ->streams[s ]->time_base;
> - tb2 = avf2->streams[s2]->time_base;
> - pkt2.pts = av_rescale_q(pkt->pts, tb, tb2);
> - pkt2.dts = av_rescale_q(pkt->dts, tb, tb2);
> - pkt2.duration = av_rescale_q(pkt->duration, tb, tb2);
> - pkt2.stream_index = s2;
> -
> - if ((ret = av_apply_bitstream_filters(avf2->streams[s2]->codec, &pkt2,
> - tee->slaves[i].bsfs[s2])) < 0 ||
> - (ret = av_interleaved_write_frame(avf2, &pkt2)) < 0) {
> +
> + if ((ret = tee_process_packet(avf2, tee->slaves[i].bsfs[s2], &pkt2,
> + tb, s2)) < 0) {
> ret = tee_process_slave_failure(avf, i, ret);
> if (!ret_all && ret < 0)
> ret_all = ret;
Regards,
--
Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160511/768f672a/attachment.sig>
More information about the ffmpeg-devel
mailing list