[FFmpeg-soc] [soc] libavsequencer [PATCH 05/08] Instrument handling public API header file.

[Vitor Sessak]

On 07/13/2010 11:13 PM, Sebastian Vater wrote:
> Vitor Sessak a écrit :
>> On 07/13/2010 10:19 PM, Sebastian Vater wrote:
>>> Vitor Sessak a écrit :
>>>> On 07/11/2010 10:07 PM, Sebastian Vater wrote:
>>>>> /**
>>>>>    * Envelope structure used by instruments to apply volume / panning
>>>>>    * or pitch manipulation according to an user defined waveform.
>>>>>    * New fields can be added to the end with minor version bumps.
>>>>>    * Removal, reordering and changes to existing fields require a major
>>>>>    * version bump.
>>>>>    */
>>>>> typedef struct AVSequencerEnvelope {
>>>>
>>>> Why not just
>>>>
>>>> typedef struct {
>>>>
>>>> ?
>>>
>>> This is a point, where I'm not sure right now:
>>> Is typedef struct { .. } AVSequencerEnevelope;
>>>
>>> exactly the same as:
>>>
>>> typedef struct AVSequencerEnvelope { .. } AVSequencerEnvelope;
>>
>> No. With
>>
>> typedef struct AVSequencerEnvelope { .. } AVSequencerEnvelope;
>>
>> both
>>
>> func() {
>> struct AVSequencerEnvelope a;
>> }
>> and
>> func() {
>> AVSequencerEnvelope a;
>> }
>>
>> compile with no errors while with
>>
>> typedef struct { .. } AVSequencerEnevelope;
>>
>> only
>>
>> AVSequencerEnvelope a;
>>
>> is valid. I don't think allowing two similar redundant types are a
>> good idea.
>
> I prefer let the programmer accessing the structures the way:
> Some prefer to write:
> struct AVSequencerEnvelope a;
>
> and some just:
> AVSequencerEnvelope a;
>
> Also the structs seen in FFmpeg use the way I use now, so I thought, to
> be compatible with remaining FFmpeg I should do the same.

Ugh, indeed (and we risk to start a bikeshed thread) ;)

> Original TuComposer didn't even use typedef's at all, the only way to
> access them was a struct TuComposerInstrEnvelope, etc.
>
> I also prefer the way to provide the target programmer more freedom than
> shrinking it, despite the fact that writing the header the way I did
> doesn't require much amount of time.
>
> This, however, is just a target-programmer-user-friendly purpose and
> wouldn't interfere with player.c by changing that, since I replaced all
> struct AVSequencer* with simply AVSequencer*
>
> However, changing that again, would require extra work by additionally
> making it a bit uncomfortable to target programmers. So if you are not
> piecy on this, I would keep is it at now, I will leave that decision
> completely to you, though. If you want me to remove that, I'll do.

Leave it as is. Maybe it is just my personal taste and not very 
important ATM.

-Vitor