On 19/11/14 18:42, Vittorio Giovara wrote:
> On Wed, Nov 19, 2014 at 11:00 AM, Luca Barbato <[email protected]> wrote:
>> On 19/11/14 07:22, Vittorio Giovara wrote:
>>> On Tue, Nov 18, 2014 at 2:27 PM, Vittorio Giovara
>>> <[email protected]> wrote:
>>>> On Tue, Nov 18, 2014 at 10:00 AM, Luca Barbato <[email protected]> wrote:
>>>>> On 18/11/14 09:16, Hendrik Leppkes wrote:
>>>>>> On Tue, Nov 18, 2014 at 8:56 AM, Luca Barbato <[email protected]> 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.
For public function sure, for small utility functions that will change
totally in the next month or two, I'd say *discussing* is overkill.
> 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
Sounds good, please write down a wiki page about it so we won't have to
dig the mailing list for those =P
> 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)?
We commonly use function() and function2() for the variant with
additional arguments.
lu
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
