[FFmpeg-devel] avformat/aiffdec: AIFF fix in case of ANNO

Submitted by Eduard Sinelnikov on Jan. 8, 2018, 2:03 p.m.

Details

Message ID 1515420220-9540-1-git-send-email-endushka@gmail.com
State New
Headers show

Commit Message

Eduard Sinelnikov Jan. 8, 2018, 2:03 p.m.
From: Eduard Sinelnikov <endushka@gmail.com>

Apple's AIFF protocol clearly states that each chucnk which is odd sized a padding should be added.
In the old version of aiffdec adding of padding was done in `get_meta`. And in case of unknown chunk name it was done in defalut case.
The new version has deleted the padding in default case and added padding adding after the switch.
But the new version didn't removed the padding adding in the `get_meta` function so in some cases padding was added twice which leaded to a bug.
---
 libavformat/aiffdec.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Michael Niedermayer Jan. 9, 2018, 8:53 p.m.
On Mon, Jan 08, 2018 at 02:03:40PM +0000, endushka@gmail.com wrote:
> From: Eduard Sinelnikov <endushka@gmail.com>
> 
> Apple's AIFF protocol clearly states that each chucnk which is odd sized a padding should be added.
> In the old version of aiffdec adding of padding was done in `get_meta`. And in case of unknown chunk name it was done in defalut case.
> The new version has deleted the padding in default case and added padding adding after the switch.
> But the new version didn't removed the padding adding in the `get_meta` function so in some cases padding was added twice which leaded to a bug.

do you have a testcase you can share that shows the probnlem ?
or did you find the issue thorugh code review ?

[...]
Eduard Sinelnikov Jan. 10, 2018, 7:03 a.m.
Yes, the problem found from playing the file AIFF file you can download it
from: https://www.datafilehost.com/d/60337fcd


On Tue, Jan 9, 2018 at 10:53 PM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Mon, Jan 08, 2018 at 02:03:40PM +0000, endushka@gmail.com wrote:
> > From: Eduard Sinelnikov <endushka@gmail.com>
> >
> > Apple's AIFF protocol clearly states that each chucnk which is odd sized
> a padding should be added.
> > In the old version of aiffdec adding of padding was done in `get_meta`.
> And in case of unknown chunk name it was done in defalut case.
> > The new version has deleted the padding in default case and added
> padding adding after the switch.
> > But the new version didn't removed the padding adding in the `get_meta`
> function so in some cases padding was added twice which leaded to a bug.
>
> do you have a testcase you can share that shows the probnlem ?
> or did you find the issue thorugh code review ?
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> It is dangerous to be right in matters on which the established authorities
> are wrong. -- Voltaire
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Michael Niedermayer Jan. 11, 2018, 12:18 a.m.
On Wed, Jan 10, 2018 at 09:03:00AM +0200, Eduard Sinelnikov wrote:
> Yes, the problem found from playing the file AIFF file you can download it
> from: https://www.datafilehost.com/d/60337fcd

thanks, i will apply the patch

[...]

Patch hide | download patch | download mbox

diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c
index 99e05c7..20decc5 100644
--- a/libavformat/aiffdec.c
+++ b/libavformat/aiffdec.c
@@ -81,11 +81,10 @@  static void get_meta(AVFormatContext *s, const char *key, int size)
             av_free(str);
             return;
         }
-        size += (size&1)-res;
+        size -= res;
         str[res] = 0;
         av_dict_set(&s->metadata, key, str, AV_DICT_DONT_STRDUP_VAL);
-    }else
-        size+= size&1;
+    }
 
     avio_skip(s->pb, size);
 }