On Sun, Nov 1, 2015 at 9:35 AM, wm4 <[email protected]> wrote: > On Sat, 31 Oct 2015 12:37:17 -0400 > Ganesh Ajjanagadde <[email protected]> wrote: > >> On Sat, Oct 31, 2015 at 12:10 PM, wm4 <[email protected]> wrote: >> > On Sat, 31 Oct 2015 17:08:07 +0100 >> > Luca Barbato <[email protected]> wrote: >> > >> >> On 31/10/15 14:36, wm4 wrote: >> >> > I've got some m4a samples that had jpeg cover art marked as png. Since >> >> > these files were supposedly written by iTunes, and other software can >> >> > read it (e.g. clementine does), this should be worked around. >> >> > >> >> > Since png has a very simple to detect header, while it's apparently a >> >> > real pain to detect jpeg in the general case, try to detect png and >> >> > assume jpeg otherwise. Not bothering with bmp, as I have no test case. >> >> > --- >> >> > libavformat/mov.c | 8 ++++++++ >> >> > 1 file changed, 8 insertions(+) >> >> > >> >> > diff --git a/libavformat/mov.c b/libavformat/mov.c >> >> > index 95dc1ee..9532213 100644 >> >> > --- a/libavformat/mov.c >> >> > +++ b/libavformat/mov.c >> >> > @@ -200,6 +200,14 @@ static int mov_read_covr(MOVContext *c, >> >> > AVIOContext *pb, int type, int len) >> >> > if (ret < 0) >> >> > return ret; >> >> > >> >> > + if (pkt.size >= 8 && id != AV_CODEC_ID_BMP) { >> >> > + if (AV_RB64(pkt.data) == 0x89504e470d0a1a0a) { >> >> > + id = AV_CODEC_ID_PNG; >> >> > + } else { >> >> > + id = AV_CODEC_ID_MJPEG; >> >> > + } >> >> > + } >> >> > + >> >> > st->disposition |= AV_DISPOSITION_ATTACHED_PIC; >> >> > >> >> > st->attached_pic = pkt; >> >> > >> >> >> >> +0: I'm not against it, an INT64_C() is needed for the >> >> 0x89504e470d0a1a0a constant. >> > >> > True. I just copy and pasted this from ffmpeg's id3v2.c (similar case - >> > cover art with wrong codec). Would a ULL suffix be good enough? >> >> Maybe useful to use libavcodec/png.h which #define's PNGSIG? This has >> added benefit that changing (to e.g ULL) there will propagate to other >> usages of this magic constant (such as libavformat/img2dec). > > It doesn't.
I meant of course if they start using PNGSIG instead of copying over the magic constant. I still strongly encourage it. > > The other issue (the 64 bit constant) was discussed to death, and it > turned out that the code is ok as it is. It is, but it is not ideal, since hex constants that represent such magic values are often better treated as unsigned. > > So I guess this patch doesn't need further changes. Perfectly fine as is. > _______________________________________________ > libav-devel mailing list > [email protected] > https://lists.libav.org/mailman/listinfo/libav-devel _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
