| Message ID | 1471943019-14136-6-git-send-email-erkki.seppala.ext@nokia.com |
|---|---|
| State | Superseded |
| Headers | show |
2016-08-23 18:03 GMT+09:00 <erkki.seppala.ext@nokia.com>: > From: Erkki Seppälä <erkki.seppala.ext@nokia.com> > > Signed-off-by: Erkki Seppälä <erkki.seppala.ext@nokia.com> > Signed-off-by: OZOPlayer <OZOPL@nokia.com> > --- > libavformat/movenc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index 8c4252d..34bc235 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -4203,6 +4203,8 @@ static int mov_write_ftyp_tag(AVIOContext *pb, > AVFormatContext *s) > > avio_wb32(pb, minor); > > + if (mov->mode == MODE_MP4 && mov->major_brand) > + ffio_wfourcc(pb, mov->major_brand); /* write major brand as a > compatible brand */ > WTF. libavformat has not listed all compatible brands? This is a wrong approach if what David Singer (Apple) says is correct. The major_brand always be written into compatible_brands in ISOBMFF. > if (mov->mode == MODE_MOV) > ffio_wfourcc(pb, "qt "); > else if (mov->mode == MODE_ISM) { > -- > 2.7.4 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Hi! 2016-08-23 11:03 GMT+02:00 <erkki.seppala.ext@nokia.com>: > + if (mov->mode == MODE_MP4 && mov->major_brand) > + ffio_wfourcc(pb, mov->major_brand); /* write major brand as a compatible brand */ How can I reproduce the issue this is trying to fix? Carl Eugen
Hello, On 08/23/2016 10:00 PM, Carl Eugen Hoyos wrote: >> + if (mov->mode == MODE_MP4 && mov->major_brand) >> + ffio_wfourcc(pb, mov->major_brand); /* write major brand as a compatible brand */ > > How can I reproduce the issue this is trying to fix? The issue we were fixing was that in the presence of custom major brand (the option "brand" is set) the custom major brand did not end up in the compatible brands. (In retrospect, this would have been a great commit message..) Prompted by your comment, we reviewed the specification, and it does not seem like that the standard requires this functionality - but it doesn't outright prohibit it either. An alternative for our use case would be adding the option "compatible_brands" for setting custom compatible brands from the client code. It would probably either replace all custom brands with the ones provided, or it would need to collect the custom brands in a list in order to remove duplicates (though I imagine allowing duplicates would only be a esthetic flaw). Thanks for the review and input!
Hi! 2016-08-24 16:53 GMT+02:00 Erkki Seppälä <erkki.seppala.ext@nokia.com>: > > On 08/23/2016 10:00 PM, Carl Eugen Hoyos wrote: >>> >>> + if (mov->mode == MODE_MP4 && mov->major_brand) >>> + ffio_wfourcc(pb, mov->major_brand); /* write major brand as a >>> compatible brand */ >> >> How can I reproduce the issue this is trying to fix? > > The issue we were fixing was that in the presence of custom major brand (the > option "brand" is set) the custom major brand did not end up in the > compatible brands. Thanks for explaining! (I couldn't find the option yesterday.) > (In retrospect, this would have been a great commit > message..) Yes;-) > An alternative for our use case would be adding the option > "compatible_brands" for setting custom compatible brands from the client > code. The advantage is that I believe users have asked for this. No more comments from me, Carl Eugen
On 08/23/2016 06:35 PM, Yusuke Nakamura wrote: >> + if (mov->mode == MODE_MP4 && mov->major_brand) >> + ffio_wfourcc(pb, mov->major_brand); /* write major brand as a >> compatible brand */ >> > > WTF. libavformat has not listed all compatible brands? This is a wrong > approach if what David Singer (Apple) says is correct. The major_brand > always be written into compatible_brands in ISOBMFF. Hmm, so you're saying it should be written unconditionally? It seems that in many cases it does happen so that the 4cc (ie. mp41) ends up being written into the compatible brands as well. But this does not happen if the major_brand is explicitly passed as an option. I have a new patch with a better commit message (but same content).
diff --git a/libavformat/movenc.c b/libavformat/movenc.c index 8c4252d..34bc235 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -4203,6 +4203,8 @@ static int mov_write_ftyp_tag(AVIOContext *pb, AVFormatContext *s) avio_wb32(pb, minor); + if (mov->mode == MODE_MP4 && mov->major_brand) + ffio_wfourcc(pb, mov->major_brand); /* write major brand as a compatible brand */ if (mov->mode == MODE_MOV) ffio_wfourcc(pb, "qt "); else if (mov->mode == MODE_ISM) {