On Wed, Nov 19, 2014 at 11:00 AM, Luca Barbato <lu_z...@gentoo.org> wrote:
> On 19/11/14 07:22, Vittorio Giovara wrote:
>> On Tue, Nov 18, 2014 at 2:27 PM, Vittorio Giovara
>> <vittorio.giov...@gmail.com> wrote:
>>> On Tue, Nov 18, 2014 at 10:00 AM, Luca Barbato <lu_z...@gentoo.org> wrote:
>>>> On 18/11/14 09:16, Hendrik Leppkes wrote:
>>>>> On Tue, Nov 18, 2014 at 8:56 AM, Luca Barbato <lu_z...@gentoo.org> wrote:
>>>>>
>>>>>> On 18/11/14 00:57, Kieran Kunhya wrote:
>>>>>>>> I added ff_combine_packet since it reduces the number of lines
>>>>>>>> and makes the code slightly less verbose.
>>>>>>>
>>>>>>> This is very confusing since ff_combine_packet suggests an AVPacket is
>>>>>> involved.
>>>>>>
>>>>>> It will and it is not less confusing that using "frame" since it is a
>>>>>> frame-worth-packet what you get out of this machinery anyway.
>>>>>>
>>>>>>
>>>>> But since you are introducing new API, an argument like "the old API also
>>>>> was confusing, so this one might as well be" is really not something you
>>>>> should be using. ;)
>>>>> Dispense with the confusion in the new API!
>>>>
>>>> It is *less* confusing, that's the whole point.
>>>
>>> Since this is just a name replacement, can anyone suggest a better
>>> name for both functions?
>>
>> On IRC we were suggested ff_try_combine_frame(), does this name appeal
>> to everyone?
>> If so, I'll start queueing the cosmetics first and then resend the
>> renamed patches.
>>
>
> No, it is wrong.
>
> To reinstate here:
>
> What we are doing is collate data fragment that might be scattered
> across multiple packets and will end in a single packet containing an
> amount of data that could be fed to the decoder and would get back a
> full decoded frame (ignoring B-frames and friends).
>
> The function has two failure paths, one fatal (no memory) and one non
> fatal (no enough data to make a whole packet).
>
> We have two variants to reduce the amount of boilerplate code.
>
>
> So the name (for a pair of internal symbols) could follow the
>
> ff_ + ${action} + _ + ${object}
>
> **action** can be anything among `combine`, `collate` and such
>
> **object** can be `data`, `data_packet` or `packet_data` and so forth
>
>
> There is no `try`. The second variant can use the `2` suffix.
>
>
> I'm delighted for the strive to perfection shown here btw.
>
> lu
I think
ff_ + ${action} + _ + ${object}
is discouraged as any editor listing the functions will order them by
action rather than by object.
A better grouping could be
ff_ + ${object} + _ + ${action}
so that all functions pertaining to the same group are listed closely together.
Eg. ff_packet_combine, ff_packet_destroy etc, rather than
ff_get_packet, ff_get_frame, ff_get_somethingelse
Having said that, i think that a ff_packet_combine2 is confusing to
the user, can we find a better candidate name (in a sensible period of
time)?
--
Vittorio
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
