[FFmpeg-devel] [PATCH 02/13] lavf: update auto-bsf to new BSF API

[Nicolas George]

L'octidi 8 messidor, an CCXXIV, Marton Balint a écrit :
> I thought your primary concern was the overhead which was discussed.

It was only one reason amongst several; the other reasons are pretty much
the usual benefits in the discussion between designing a new API for a new
feature or fitting it in an existing API: more freedom in the design means
simpler code, simpler API, static type safety.

> As far as I know the only thing left is the "cleanness". A totally separate
> list API would involve a lot of code duplication (provide the N:M API for
> lists, and provide the same fields to a BSF list which are already available
> as part of a BSF context. (time_base, codec parameters)

Two functions and four fields, I would say it is rather reasonable.

> And we only need the list API for the configuration anyway. So what if we
> destroy the list after the configuration is done, and use it as a simple BSF
> afterwards?
> 
> Is it OK if the following is implemented?
> 
> AVBSFList *list = av_bsf_list_alloc()
> 
> // Append as many filters as you want...
> av_bsf_list_append(AVBSFList *, AVBSFContext *)
> 
> // Destroy the list structure and return the BSF context which is the //
> container list filter we discussed, and which can be used as any other //
> freshly allocated BSF context
> av_bsf_list_finalize(AVBSFList **list, AVBSFContext **ctx)

I would say it reaps about half or three quarters of the benefits of a clean
API (mostly: the type safety), at the cost of being somewhat confusing.

But this is assuming it can be achieved without costly overhead (malloc()s)
in the empty list case. With a completely new API, it can be achieved quite
easily. With a wrapping bsf, it can certainly be achieved (it is just an
API, after all), but I am not sure it can be done without reworking the
internal design quite a lot.

Well, looking at the code, I am thinking that the current design is flawed:
the extra alloc in ff_bsf_get_packet() seems completely useless, and could
be removed as is without any other change in the current code, because all
current filters are 1:1, it would be different for future 1:N filters. Maybe
implementing ff_bsf_peek_packet() and using it to replace
ff_bsf_get_packet() in 1:1 cases would do the trick.

In summary: I am ok with this version IFF it works without malloc() overhead
for empty lists and is reasonably simple.

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/20160628/bd973dd3/attachment.sig>