On Sat, 11 Jun 2016, Nicolas George wrote:

Instead of adding a totally new API, I suggest to use a "container"
bitstream filter called 'list', which will instantiate the "child" bitstream
filters and pump the packets through them.

Thanks to the N:M nature of the new BSF api, this should be achievable, and
we can completely hide for the API user if we are using a single bitstream
filter, or a chain (list) of several bitstream filters.

I had the same idea, and it is not without merits, but I no longer think it
is the best course to go.

The main merit of the idea is to keep the API identical, and it is mostly
useful for compatibility with the fork. Since this is no longer a primary
goal, other considerations must prevail.

As I observed benchmarking the null bsf, the overhead of a dummy bsf is not
entirely negligible, mostly due to a few mallocs. Better avoid it if
possible.

Maybe I am missing something, but as far as I see the alloc is due to the alloc in ff_bsf_get_packet. We could create a similar function ff_bsf_get_packet2(AVBSFContext *ctx, AVPacket *pkt) which does not allocate a packet but simply moves the ref to pkt. Then this can be called with the output packet pointer, so no allocation will be requried. And this function can be used for performance sensitive places instead of the original.

The mentioned 'list' bitstream filter could have an AVOption string
parameter called list which it can parse on init to create the bitstream
filter chain. The list string would follow the ffmpeg bsf list syntax:
filter1[=opt1=str1/opt2=str2][,filter2]

For that, I say no: it must have a proper API. We can have a parsing
functions for user-provided strings describing list of filters, but nowadays
most filter lists are created by applications or the library, and for that
we do not want a round-trip to a string with all the problems that entails
(quoting, memory allocation or arbitrary limits), this kind of madness is
already present in too many places.

The API itself could be only:

   ret = av_bsf_list_append(list, filter, &ctx);

The append functions returns the newly created filter, so we can set options
on it if necessary. Then the filters are inited, either by calling
av_bsf_init() on each of them or by having av_bsf_list_init_all(), either
solutions has merits of its own.

Note that this solution can work with a container filter too. I would prefer
that list is its own dedicated type, maybe AVBSFList, but if it is an
AVBSFContext, av_bsf_list_append() can assume it is a container filter and
access its private structure.

I think we need to support both ways to configure the bsf list. What the tee muxer, or ffmpeg.c would use, is the text based configuration, therefore it's worth adding the capability of parsing such lists.

So what we need, on top of what I proposed, is an additional API what you proposed (av_bsf_list_append), and which can be used to add filters to the list, after we allocated the list filter, but before we initialized it.

This way the user can decide which configuration method - API based or text based - he will use.

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to