[FFmpeg-devel] [PATCH] Split common code for MLP to mlp.[ch].

Ramiro Polla ramiro.polla
Wed Aug 13 15:13:04 CEST 2008


Hello,

I replied to this e-mail with two sets of big patches and it was too
big for the ML. I'm sending them again separately.

On Wed, Aug 13, 2008 at 9:38 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> SSOn Wed, Aug 13, 2008 at 12:38:07AM -0300, Ramiro Polla wrote:
>> Hello,
>>
>> Attached patches remove common code from the MLP encoder in the SoC
>> repo and the decoder (and the parser) and puts it in common files.
>>
>> To commit, the split to mlp.[ch] would be made with svn cp from
>> mlpdec.c. I attached them as the resulting file to easy reviewing (or
>> is it better to have the diffs to mlpdec.c?).
>
> you could just attach both they are both usefull for reviewing

I'm attaching them as diffs again. One for each file, but to be
committed all at once. What changed is that I removed the duplicate
comment for both checksum functions in mlp.h. Also I copied with the
*/ in the same line, since that's how it is in mlpdec.

I'll post a followup to this reply with the second set of patches.

> as is my awnser to the patch is
> its ok under the condition that it just moves stuff around (hard to
> tell as the new files are not attached as diff)
> and that the issue pointed out below is fixed
>
> [...]
>
>> } ChannelParams;
>>
>> /** Tables defining the Huffman codes.
>>  *  There are three entropy coding methods used in MLP (four if you count
>>  *  "none" as a method). These use the same sequences for codes starting with
>>  *  00 or 01, but have different codes starting with 1. */
>>
>> extern const uint8_t ff_mlp_huffman_tables[3][18][2];
>
> the */ should be on a line of its own so adding more text does not
> need to change the previous last line

Do you prefer if I change all comments to end with */ in the next
line? I'll do that as a separate commit then.

> also there should be more \n above than below, as is the comment is exactly
> between 2 things and that looks a little odd. moving the */ wil likely be
> enough though when theres no empty line after it..

Maybe it's just because I'm tired, but I didn't quite understand this
part. More \n above than below what?

Ramiro Polla
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mlp.c.diff
Type: text/x-diff
Size: 40822 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080813/ddcbf36c/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mlp.h.diff
Type: text/x-diff
Size: 40070 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080813/ddcbf36c/attachment-0001.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mlpdec.c.diff
Type: text/x-diff
Size: 8298 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080813/ddcbf36c/attachment-0002.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mlp_parser.c.diff
Type: text/x-diff
Size: 1868 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080813/ddcbf36c/attachment-0003.diff>



More information about the ffmpeg-devel mailing list