Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-26 Thread Tinglin Liu
On Mon, Oct 26, 2015 at 8:06 AM, Derek Buitenhuis < derek.buitenh...@gmail.com> wrote: > On 10/23/2015 7:41 PM, Tinglin Liu wrote: > > Derek, would you do the amend and push? Let me know if you need me to > > resend an amended patch. Thanks. > > Amended and pushed. > > As before: Is there a

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-26 Thread Derek Buitenhuis
On 10/23/2015 7:41 PM, Tinglin Liu wrote: > Derek, would you do the amend and push? Let me know if you need me to > resend an amended patch. Thanks. Amended and pushed. As before: Is there a sample somewhere we can add a FATE test for? - Derek ___

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-25 Thread wm4
On Sun, 25 Oct 2015 12:39:08 +0100 Nicolas George wrote: > Le quartidi 4 brumaire, an CCXXIV, Derek Buitenhuis a écrit : > > Perhaps wm4 or Michael or someone can chime in... I don't know of a single > > good > > way to handle this. Just failing if the locale has a radix point

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-25 Thread Derek Buitenhuis
On 10/25/2015 12:06 PM, wm4 wrote: > I think we don't need to block the patch on this issue anymore, because: > 1. We don't have any workarounds in place this patch could use, > 2. Lots of other code is probably affected anyway I agree, it is beyond the scope of this patch. - Derek

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-25 Thread Derek Buitenhuis
On 10/23/2015 7:41 PM, Tinglin Liu wrote: > ​ > http://stackoverflow.com/questions/3457968/snprintf-simple-way-to-force-as-radix > ​ > Here it mentioned using the setlocale() function, but I didn't find any > examples elsewhere though > > Derek, would you do the amend and push? Let me know if

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-25 Thread Nicolas George
Le quartidi 4 brumaire, an CCXXIV, Derek Buitenhuis a écrit : > Perhaps wm4 or Michael or someone can chime in... I don't know of a single > good > way to handle this. Just failing if the locale has a radix point as a comma > instead > of a period seems less bad than mucking with the

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-25 Thread Nicolas George
Le quartidi 4 brumaire, an CCXXIV, wm4 a écrit : > Well, I'm not sure if this only affects LC_NUMERIC and maybe LC_COLLATE? I grant you that LC_PAPER or LC_TELEPHONE are probably harmless; they were added after I made my mental list. It should work by whitelist, not by blacklist. > On the other

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-23 Thread Tinglin Liu
On Thu, Oct 22, 2015 at 4:57 PM, Derek Buitenhuis < derek.buitenh...@gmail.com> wrote: > On 10/23/2015 12:19 AM, wm4 wrote: > > Wrong, snprintf() always returns the number of characters the string > > would have been, even if the buffer size is smaller. > > That'll teach me to reply past

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-22 Thread Tinglin Liu
Thank you Derek for the reply. Basically the metadata is stored in the structure like: |--meta |hdlr |keys |ilst 1) Firstly, we check if the handler type in the metadata handler atom is ‘mdta’. If it is, we set found_hdlr_mdta in ​ MOVContext be 1. 2) The key and value are stored

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-22 Thread Derek Buitenhuis
On 10/22/2015 11:04 PM, Tinglin Liu wrote: > +} else if (data_type == 23 && str_size >= 4) { // BE float32 > +union av_intfloat32 val; > +val.i = avio_rb32(pb); I found we have a function to to this: av_int2float(). > +if (snprintf(str,

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-22 Thread wm4
On Fri, 23 Oct 2015 00:10:20 +0100 Derek Buitenhuis wrote: > On 10/22/2015 11:04 PM, Tinglin Liu wrote: > > +} else if (data_type == 23 && str_size >= 4) { // BE float32 > > +union av_intfloat32 val; > > +val.i = avio_rb32(pb); > >

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-22 Thread Derek Buitenhuis
On 10/22/2015 11:28 PM, Tinglin Liu wrote: > 2) The key and value are stored separately for each key-value pair. 'keys' > atom stores the key name table, while 'ilst' atom stores the values > corresponding to the indices in the key table. And since they are stored in > two different atoms, I have

Re: [FFmpeg-devel] [PATCH] Adds support parsing the QuickTime Metadata Keys.

2015-10-22 Thread Derek Buitenhuis
On 10/23/2015 12:19 AM, wm4 wrote: > Wrong, snprintf() always returns the number of characters the string > would have been, even if the buffer size is smaller. That'll teach me to reply past midnight. I am dumb at night. > Also, shouldn't this use some av_ wrapper? What about locale issues? >