[FFmpeg-soc] [soc] Questions on patch order, threads and review comments

S.N. Hemanth Meenakshisundaram smeenaks at ucsd.edu
Tue Jul 6 11:45:09 CEST 2010


Hi Stefano,

I have a few questions on the order of patches and on some of your review
comments from earlier:

>> >
>> >Here is the working af_resample.c and associated Makefile and
>> >allfilters.c changes.
>> >
>> >[...]
>>
>>
>
>> -#define AV_PERM_READ     0x01   ///< can read from the buffer
>> -#define AV_PERM_WRITE    0x02   ///< can write to the buffer
>> -#define AV_PERM_PRESERVE 0x04   ///< nobody else can overwrite the
>> buffer
>> -#define AV_PERM_REUSE    0x08   ///< can output the buffer multiple
>> times, with the same contents each time
>> -#define AV_PERM_REUSE2   0x10   ///< can output the buffer multiple
>> times, modified each time
>
> The AV_PERM_* moving can go to a separate patch.

Should I submit the AV_PERM_* patch first (move it out of AVFilterPicRef
structure) and then the the audio framework changes patch? Or should I
focus on the audio filter patches and define the PERM macros twice (inside
AVFilterPicRef and AVFilterSamplesRef structures).


>
>> -    link->format = link->in_formats->formats[0];
>> +    if (link->type == AVMEDIA_TYPE_VIDEO)
>> +        link->format = link->in_formats->formats[0];
>> +    else if (link->type == AVMEDIA_TYPE_AUDIO)
>> +        link->aformat = link->in_formats->aformats[0];
>
> simpler: link->format = type == VIDEO ? link->in_formats->formats[0] :
> link->in_formats->aformats[0];

Here I need to assign to link->format in case of video and link->aformat
in case of audio. How can I get this done using the ?: form above?


>
>> +#define CMP_AND_ADD(acount, bcount, afmt, bfmt, retfmt) { \
>> +    for(i = 0; i < acount; i++) \
>> +        for(j = 0; j < bcount; j++) \
>> +            if(afmt[i] == bfmt[j]) \
>> +                retfmt[k++] = afmt[i]; \
>> +}
>> [...]
>>
>
> This refactoring is nice but please split it in a separate patch (and
> separate thread).

Again, should I submit the CMP_AND_ADD refactoring patch first before the
audio filter patches?


I should start a new thread for each patch - is that right?

Also, I now have some patches based on your comments and some of the
additions for channel layout conversion etc.

Should I send these as patches of patches or send the entire patch each
time starting from whatever is not yet in the soc svns? Patches of patches
would be difficult to read while sending the entire patch again hides the
new changes.

Finally, Wikipedia says the downmix formulae for ac3 to stereo are:

Left Only = Left (Front) + -3dB*Center + att*LeftSide

Right Only = Right (Front) + -3dB*Center + att*RightSide

where att = -3dB, -6dB or -9dB.

The article has no references. Can I just go ahead and use this? It seems
to work well for a 5.0 track I have with att = -9dB.

Regards,
Hemanth


Regards,
Hemanth







More information about the FFmpeg-soc mailing list