[FFmpeg-devel] [PATCH 4/6] mp3enc: add support for writing UTF-16 tags

[Måns Rullgård]

"Ronald S. Bultje" <rsbultje at gmail.com> writes:

> Hi,
>
> On Fri, Jan 21, 2011 at 2:18 PM, Anton Khirnov <anton at khirnov.net> wrote:
>> + ? ?void (*put)(ByteIOContext*, const char*) = avio_put_str;
>> + ? ?ByteIOContext *dyn_buf;
>> + ? ?if (url_open_dyn_buf(&dyn_buf) < 0)
>> + ? ? ? ?return 0;
>
> return -1? This is an error.

It seems deliberate.  The return value is simply interpreted as the
number of bytes written.  Returning -1 here would cause bad things.

The question is whether silently ignoring errors here is appropriate.

>> + ? ?put_byte(dyn_buf, enc);
>> + ? ?if (enc == ID3v2_ENCODING_UTF16BOM) {
>> + ? ? ? ?put_le16(dyn_buf, 0xFEFF); ? ? ?/* BOM */
>> + ? ? ? ?put = avio_put_str16le;
>> + ? ?}
> [..]
>
> } else {
>     put = avio_put_str;
> }

It is initialised in the declaration.  I too had to look around before
I found it, probably because the declarations are already a bit
chaotic (as function pointers tend to get).  An explicit else might
make it easier to read quickly.

-- 
M?ns Rullg?rd
mans at mansr.com