Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/28/2016 12:22 AM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 09:24:14PM +0100, Mats Peterson wrote: On 02/27/2016 09:21 PM, Mats Peterson wrote: On 02/27/2016 08:42 PM, Reimar Döffinger wrote: Well, then we know I can only blame Mats for "only" making it better instead of perfect :) ___ Well, before my patches, there was no palette inclusion whatsoever in AVI or QuickTime on muxing. Anyway, you're right about the stream copying not working. Just tried it here. Another issue to take care of... phew. Solving the stream copying issues obviously requires using side data packets in the muxers, correct me if I'm wrong. Well, that is one option. It's obviously the only option considering the current state of FFmpeg. Then again, I don't see the point with stream copying raw palettized data, since the strides are different between the file formats. Although I agree that the palette transfer should work. It's up to the user to determine if what he/she is doing is sensible when using stream copy for raw palettized data. It's already stated in the FFmpeg documentation that stream copy can be problematic. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On Sat, Feb 27, 2016 at 09:24:14PM +0100, Mats Peterson wrote: > On 02/27/2016 09:21 PM, Mats Peterson wrote: > >On 02/27/2016 08:42 PM, Reimar Döffinger wrote: > >>Well, then we know I can only blame Mats for "only" making it > >>better instead of perfect :) > >>___ > > > >Well, before my patches, there was no palette inclusion whatsoever in > >AVI or QuickTime on muxing. Anyway, you're right about the stream > >copying not working. Just tried it here. Another issue to take care > >of... phew. > > > > Solving the stream copying issues obviously requires using side data packets > in the muxers, correct me if I'm wrong. Well, that is one option. It is also possible for the demuxers to (possibly even just in addition) append the palette - but I think that is problematic when palette isn't provided for every frame. Probably better would be for muxers to have a little magic function. const uint8_t *ff_update_get_palette(AVFormatContext *, AVFrame *, bool); That function would then do something like (possibly a bit overkill as I describe it): 1) Check if it's a paletted raw format and the palette is appended. If so extract it, possibly expand it, store it in the context and return the pointer. 2) Check if there is side data palette, and if so store it in the context too and return the pointer 3) If palette is not available in any way right now, return the one we previously stored in the palette if any. The bool could then be used to configure whether the return value should be NULL instead in case of 3) or when in 1) and 2) the new palette is identical (that way it is selectable if the palette gets stored with every frame or not, it could probably even expanded to track actual changed entries). As said, that is overkill, but it shouldn't hurt to at least have possible future "nice to haves" in mind from the start. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 09:33 PM, Michael Niedermayer wrote: On Sat, Feb 27, 2016 at 08:42:25PM +0100, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 07:51:41PM +0100, Michael Niedermayer wrote: should rawvideo AVPackets palette use data[] or sidedata, honestly i do not know, but i dont think it makes a big difference even supporting both, likely only adds 3-5 lines of code or so its more a philosophical question Not sure how much was in response to my complaints, but i dont know either, i was just writing some kind of summary ... if this parts was I think that misses my point, as I agree that it doesn't matter which. What matters though is: The AVI demuxer creates AVPackets with palette in side data, and the AVI muxer supports ONLY palette appended in the data. How to fix it might be philosophical, but I think it's not philosophical and fairly clear that that's just broken. yes, though rawvideo stream copy is a bit tricky, especially when its not the same container. avi<->mov have different stride align requirements for example. Yes, I mentioned that before, regarding the stride differences. One question: For stream copying, the packets will travel unaltered from the demuxer to the muxer including the side data, right? Sorry if it sounds idiotic. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On Sat, Feb 27, 2016 at 08:42:25PM +0100, Reimar Döffinger wrote: > On Sat, Feb 27, 2016 at 07:51:41PM +0100, Michael Niedermayer wrote: > > should rawvideo AVPackets palette use data[] or sidedata, honestly i > > do not know, but i dont think it makes a big difference > > even supporting both, likely only adds 3-5 lines of code or so > > its more a philosophical question > > Not sure how much was in response to my complaints, but i dont know either, i was just writing some kind of summary ... > if this parts was I think that misses my point, as I > agree that it doesn't matter which. > What matters though is: > The AVI demuxer creates AVPackets with palette in side data, > and the AVI muxer supports ONLY palette appended in the data. > How to fix it might be philosophical, but I think it's not > philosophical and fairly clear that that's just broken. yes, though rawvideo stream copy is a bit tricky, especially when its not the same container. avi<->mov have different stride align requirements for example. We currently correct that in the muxer If we similarly also handle side data and extradata palettes in the muxer (via shared code ideally) then any input would work not sure this is the best solution [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Concerning the gods, I have no means of knowing whether they exist or not or of what sort they may be, because of the obscurity of the subject, and the brevity of human life -- Protagoras signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 09:21 PM, Mats Peterson wrote: On 02/27/2016 08:42 PM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 07:51:41PM +0100, Michael Niedermayer wrote: should rawvideo AVPackets palette use data[] or sidedata, honestly i do not know, but i dont think it makes a big difference even supporting both, likely only adds 3-5 lines of code or so its more a philosophical question Not sure how much was in response to my complaints, but if this parts was I think that misses my point, as I agree that it doesn't matter which. What matters though is: The AVI demuxer creates AVPackets with palette in side data, and the AVI muxer supports ONLY palette appended in the data. How to fix it might be philosophical, but I think it's not philosophical and fairly clear that that's just broken. Even remuxing from AVI to AVI simply won't work without a "decoder" (which is not an issue performance-wise, but it is usability wise, how should a FFmpeg user know that for paletted video and only that a lossless stream copy requires "decoding" and "encoding"?). about existing API, i suspect there arent many applications that use ffmpegs demuxers without the decoders for raw pal8, i might of course be wrong but this seems a rather uncommon case of a uncommon case. And muxers side it was all broken before mats ... Well, then we know I can only blame Mats for "only" making it better instead of perfect :) ___ Well, before my patches, there was no palette inclusion whatsoever in AVI or QuickTime on muxing. Anyway, you're right about the stream copying not working. Just tried it here. Another issue to take care of... phew. Solving the stream copying issues obviously requires using side data packets in the muxers, correct me if I'm wrong. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 08:42 PM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 07:51:41PM +0100, Michael Niedermayer wrote: should rawvideo AVPackets palette use data[] or sidedata, honestly i do not know, but i dont think it makes a big difference even supporting both, likely only adds 3-5 lines of code or so its more a philosophical question Not sure how much was in response to my complaints, but if this parts was I think that misses my point, as I agree that it doesn't matter which. What matters though is: The AVI demuxer creates AVPackets with palette in side data, and the AVI muxer supports ONLY palette appended in the data. How to fix it might be philosophical, but I think it's not philosophical and fairly clear that that's just broken. Even remuxing from AVI to AVI simply won't work without a "decoder" (which is not an issue performance-wise, but it is usability wise, how should a FFmpeg user know that for paletted video and only that a lossless stream copy requires "decoding" and "encoding"?). about existing API, i suspect there arent many applications that use ffmpegs demuxers without the decoders for raw pal8, i might of course be wrong but this seems a rather uncommon case of a uncommon case. And muxers side it was all broken before mats ... Well, then we know I can only blame Mats for "only" making it better instead of perfect :) ___ Well, before my patches, there was no palette inclusion whatsoever in AVI or QuickTime on muxing. Anyway, you're right about the stream copying not working. Just tried it here. Another issue to take care of... phew. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 08:44 PM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 07:52:52PM +0100, Mats Peterson wrote: On 02/27/2016 07:46 PM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 07:40:44PM +0100, Mats Peterson wrote: On 02/27/2016 07:18 PM, Reimar Döffinger wrote: That's not what e.g. the matroska demuxer produces, so stream copying with some generic remuxing example I am really sure that is NOT how the packets come in. Stream copying of raw palettized data seldom works well, due to different strides of AVI (4-byte aligned), QuickTime (2-byte aligned) and nut (unnaligned). "Seldom"? That's only an issue (for 8 bpp) if the width is not a multiple of 4. It's an issue for 1 bpp data as well. Not the parsing I meant :) For 8bpp it can only happen if the width is not a multiple of 4. For 1bpp it can only happen if the width is not a multiple of 16 (which is quite a bit more likely and thus I didn't want to use that one as example). ___ I know you wouldn't want to :) Never mind, it's a minor issue compared to the others, as you say yourself. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On Sat, Feb 27, 2016 at 07:52:52PM +0100, Mats Peterson wrote: > On 02/27/2016 07:46 PM, Reimar Döffinger wrote: > >On Sat, Feb 27, 2016 at 07:40:44PM +0100, Mats Peterson wrote: > >>On 02/27/2016 07:18 PM, Reimar Döffinger wrote: > >>>That's not what e.g. the matroska demuxer produces, so stream > >>>copying with some generic remuxing example I am really sure that is > >>>NOT how the packets come in. > >>> > >> > >>Stream copying of raw palettized data seldom works well, due to different > >>strides of AVI (4-byte aligned), QuickTime (2-byte aligned) and nut > >>(unnaligned). > > > >"Seldom"? That's only an issue (for 8 bpp) if the width > >is not a multiple of 4. > > It's an issue for 1 bpp data as well. Not the parsing I meant :) For 8bpp it can only happen if the width is not a multiple of 4. For 1bpp it can only happen if the width is not a multiple of 16 (which is quite a bit more likely and thus I didn't want to use that one as example). ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On Sat, Feb 27, 2016 at 07:51:41PM +0100, Michael Niedermayer wrote: > should rawvideo AVPackets palette use data[] or sidedata, honestly i > do not know, but i dont think it makes a big difference > even supporting both, likely only adds 3-5 lines of code or so > its more a philosophical question Not sure how much was in response to my complaints, but if this parts was I think that misses my point, as I agree that it doesn't matter which. What matters though is: The AVI demuxer creates AVPackets with palette in side data, and the AVI muxer supports ONLY palette appended in the data. How to fix it might be philosophical, but I think it's not philosophical and fairly clear that that's just broken. Even remuxing from AVI to AVI simply won't work without a "decoder" (which is not an issue performance-wise, but it is usability wise, how should a FFmpeg user know that for paletted video and only that a lossless stream copy requires "decoding" and "encoding"?). > about existing API, i suspect there arent many applications that > use ffmpegs demuxers without the decoders for raw pal8, i might > of course be wrong but this seems a rather uncommon case of a uncommon > case. And muxers side it was all broken before mats ... Well, then we know I can only blame Mats for "only" making it better instead of perfect :) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 07:51 PM, Michael Niedermayer wrote: On Sat, Feb 27, 2016 at 05:45:57PM +0100, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 04:15:10PM +0100, Mats Peterson wrote: On 02/27/2016 04:13 PM, Mats Peterson wrote: On 02/27/2016 04:08 PM, Mats Peterson wrote: On 02/27/2016 04:07 PM, Mats Peterson wrote: On 02/27/2016 04:00 PM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 03:57:06PM +0100, Mats Peterson wrote: On 02/27/2016 03:37 PM, Mats Peterson wrote: ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I suppose this is what you mean, Reimar. Treating the palette, if a packet contains one at the end of the video data, as being AVPALETTE_SIZE bytes exclusively. Well, actually not really. If the palette is part of input frame it should be sent as side data. I am not sure where this variant comes from. It might be that it should just be written as is. Or even if the palette needs to be split it might be necessary to auto-detect the palette size via packet size - (width*height*bits per pixel)/8. But as said, I am fairly unclear on what case that code is supposed to handle. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I agree that it should be stored in a side data packet myself normally, and that this is a somewhat weird construction. It probably has to do with the nut format originally, which stores raw palettized data after the video data in the packets. Anyway, I have accepted the facts. For the record, the new ff_reshuffle_raw_rgb() function written by Michael in lavf/rawutils.c that aligns strides properly for AVI and QuickTime, will set a CONTAINS_PAL flag if the packet size is larger than the actual video data. He has hardcoded the palette size to 1024 bytes in that file. Mats The nut format stores the PALETTE after the video data in the packets, nothing else :) In any case, on muxing, the packets will have the palette after the video data in the packets, whether it's AVI or QuickTime. Neither avienc.c or movenc.c uses any side data packets for the palette. Michael's intention has been to enable palette switching in the middle of the stream, hence storage of the palette in each packet, and AVI supports it by using the 'xxpc' chunks in the video data. It is also implemented by now. Mats Not that it couldn't be done with side data packets, though. If it doesn't support side data then the muxers are plain broken. If the nut muxer stores palette by appending it to the frames, then the demuxer should split it out into side data. Note that I am absolutely not a fan of this side data stuff, but since we already decided to do it like that then that's the way we need to go, not randomly doing one way in one place and differently in another, that just makes for unusable API. The only reasons to support "palette appended to data" are 1) There are some existing users of the FFmpeg API that rely on it. Ideally we should then change it so it works for all muxers, or the other way round warn that this is a deprecated way of doing things. 2) There are file formats that store it that way and we cannot easily split it into side data. Not sure that can really happen. palettes are a bit annoying, there are quite a few things the chain generally is demuxer -> decoder -> encoder -> muxer OR demuxer ---> muxer Thus there are 2 interfaces, the demuxer -> muxer and the decoder -> encoder interface For the decoder -> encoder interface, the palette is in AVFrame.data[1] the 8bit indexes as a width(stride) x height array in AVFrame.data[0] that part is still easy the demuxer -> muxer interface is more complex in case of non raw, that is compressed codecs the palette can be in a codec specific and inseprable format in AVPacket.data with the rest of the compressed image. but its also possible that there is no palette in AVPacket.data and instead its stored in AVPackets side data which would be filled from container specific chunks like avis PCxx or in the global extradata So even without rawvideo there already exist both sidedata and non sidedata cases additionally key frame AVPackets must together with the global extradata contain a full palette to be decodeable. some containers support storing "partial palettes", for example avis PCxx chunks can do that, so one should at keyframes store a full one but subequent non keyframes should only store the part that differs from the previous. The container specific compression like PCxx would semantically best fit into the muxer should rawvideo AVPackets palette use data[] or sidedata, honestly i do not know, but i dont think it makes a big difference even supporting both, likely only adds 3-5 lines of code or so its more a philosophical question is the palette like chroma or
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 07:46 PM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 07:40:44PM +0100, Mats Peterson wrote: On 02/27/2016 07:18 PM, Reimar Döffinger wrote: That's not what e.g. the matroska demuxer produces, so stream copying with some generic remuxing example I am really sure that is NOT how the packets come in. Stream copying of raw palettized data seldom works well, due to different strides of AVI (4-byte aligned), QuickTime (2-byte aligned) and nut (unnaligned). "Seldom"? That's only an issue (for 8 bpp) if the width is not a multiple of 4. It's an issue for 1 bpp data as well. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On Sat, Feb 27, 2016 at 05:45:57PM +0100, Reimar Döffinger wrote: > On Sat, Feb 27, 2016 at 04:15:10PM +0100, Mats Peterson wrote: > > On 02/27/2016 04:13 PM, Mats Peterson wrote: > > >On 02/27/2016 04:08 PM, Mats Peterson wrote: > > >>On 02/27/2016 04:07 PM, Mats Peterson wrote: > > >>>On 02/27/2016 04:00 PM, Reimar Döffinger wrote: > > On Sat, Feb 27, 2016 at 03:57:06PM +0100, Mats Peterson wrote: > > >On 02/27/2016 03:37 PM, Mats Peterson wrote: > > >> > > >> > > >> > > >>___ > > >>ffmpeg-devel mailing list > > >>ffmpeg-devel@ffmpeg.org > > >>http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > >> > > > > > >I suppose this is what you mean, Reimar. Treating the palette, if a > > >packet > > >contains one at the end of the video data, as being AVPALETTE_SIZE > > >bytes > > >exclusively. > > > > Well, actually not really. > > If the palette is part of input frame it should be sent as side > > data. > > I am not sure where this variant comes from. > > It might be that it should just be written as is. > > Or even if the palette needs to be split it might be > > necessary to auto-detect the palette size via > > packet size - (width*height*bits per pixel)/8. > > But as said, I am fairly unclear on what case that > > code is supposed to handle. > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > >>> > > >>>I agree that it should be stored in a side data packet myself normally, > > >>>and that this is a somewhat weird construction. It probably has to do > > >>>with the nut format originally, which stores raw palettized data after > > >>>the video data in the packets. Anyway, I have accepted the facts. For > > >>>the record, the new ff_reshuffle_raw_rgb() function written by Michael > > >>>in lavf/rawutils.c that aligns strides properly for AVI and QuickTime, > > >>>will set a CONTAINS_PAL flag if the packet size is larger than the > > >>>actual video data. He has hardcoded the palette size to 1024 bytes in > > >>>that file. > > >>> > > >>>Mats > > >>> > > >> > > >>The nut format stores the PALETTE after the video data in the packets, > > >>nothing else :) > > >> > > > > > >In any case, on muxing, the packets will have the palette after the > > >video data in the packets, whether it's AVI or QuickTime. Neither > > >avienc.c or movenc.c uses any side data packets for the palette. > > > > > >Michael's intention has been to enable palette switching in the middle > > >of the stream, hence storage of the palette in each packet, and AVI > > >supports it by using the 'xxpc' chunks in the video data. It is also > > >implemented by now. > > > > > >Mats > > > > > > > Not that it couldn't be done with side data packets, though. > > If it doesn't support side data then the muxers are plain broken. > If the nut muxer stores palette by appending it to the frames, > then the demuxer should split it out into side data. > Note that I am absolutely not a fan of this side data stuff, > but since we already decided to do it like that then that's > the way we need to go, not randomly doing one way in one > place and differently in another, that just makes for unusable > API. > The only reasons to support "palette appended to data" are > 1) There are some existing users of the FFmpeg API that rely on it. >Ideally we should then change it so it works for all muxers, or >the other way round warn that this is a deprecated way of doing >things. > 2) There are file formats that store it that way and we cannot easily >split it into side data. Not sure that can really happen. palettes are a bit annoying, there are quite a few things the chain generally is demuxer -> decoder -> encoder -> muxer OR demuxer ---> muxer Thus there are 2 interfaces, the demuxer -> muxer and the decoder -> encoder interface For the decoder -> encoder interface, the palette is in AVFrame.data[1] the 8bit indexes as a width(stride) x height array in AVFrame.data[0] that part is still easy the demuxer -> muxer interface is more complex in case of non raw, that is compressed codecs the palette can be in a codec specific and inseprable format in AVPacket.data with the rest of the compressed image. but its also possible that there is no palette in AVPacket.data and instead its stored in AVPackets side data which would be filled from container specific chunks like avis PCxx or in the global extradata So even without rawvideo there already exist both sidedata and non sidedata cases additionally key frame AVPackets must together with the global extradata contain a full palette to be decodeable. some containers support storing "partial palettes", for example avis PCxx chunks can do that, so one should at keyframes
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On Sat, Feb 27, 2016 at 07:40:44PM +0100, Mats Peterson wrote: > On 02/27/2016 07:18 PM, Reimar Döffinger wrote: > >That's not what e.g. the matroska demuxer produces, so stream > >copying with some generic remuxing example I am really sure that is > >NOT how the packets come in. > > > > Stream copying of raw palettized data seldom works well, due to different > strides of AVI (4-byte aligned), QuickTime (2-byte aligned) and nut > (unnaligned). "Seldom"? That's only an issue (for 8 bpp) if the width is not a multiple of 4. To me that sounds like a minor issue compared to "we designed our demuxers to use a format completely different from our muxers". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 07:40 PM, Mats Peterson wrote: On 02/27/2016 07:18 PM, Reimar Döffinger wrote: That's not what e.g. the matroska demuxer produces, so stream copying with some generic remuxing example I am really sure that is NOT how the packets come in. Stream copying of raw palettized data seldom works well, due to different strides of AVI (4-byte aligned), QuickTime (2-byte aligned) and nut (unnaligned). unaligned, not unnaligned. -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 07:18 PM, Reimar Döffinger wrote: That's not what e.g. the matroska demuxer produces, so stream copying with some generic remuxing example I am really sure that is NOT how the packets come in. Stream copying of raw palettized data seldom works well, due to different strides of AVI (4-byte aligned), QuickTime (2-byte aligned) and nut (unnaligned). Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 07:18 PM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 06:05:20PM +0100, Mats Peterson wrote: On 02/27/2016 05:45 PM, Reimar Döffinger wrote: Not that it couldn't be done with side data packets, though. If it doesn't support side data then the muxers are plain broken. The muxers should support side data, I agree on that. The thing is the packets come in to the AVI, QuickTime and nut muxers with the palette data appended to the video data in the packets. You never answered that actually... How do the come in that way?!? That's not what e.g. the matroska demuxer produces, so stream copying with some generic remuxing example I am really sure that is NOT how the packets come in. That's apparently how things work in FFmpeg at the moment. There is currently no palette side data used by the *FFmpeg* muxers. The palette is appended to the packets. Ask Michael for further details, he can probably explain this better than me. I can't see HOW they even COULD work right now?! Otherwise I wouldn't really care, but as far as I can tell I don't see how it could work at all but for some lucky exceptions. What cases have we actually tested to work? ___ In spite of the mixed use of side data and palette in packets, things work just fine. FFplay currently plays all palettized files correctly, and FFmpeg produces palettized files correctly, after my recent patches. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On Sat, Feb 27, 2016 at 06:05:20PM +0100, Mats Peterson wrote: > On 02/27/2016 05:45 PM, Reimar Döffinger wrote: > >>Not that it couldn't be done with side data packets, though. > > > >If it doesn't support side data then the muxers are plain broken. > > The muxers should support side data, I agree on that. The thing is the > packets come in to the AVI, QuickTime and nut muxers with the palette data > appended to the video data in the packets. You never answered that actually... How do the come in that way?!? That's not what e.g. the matroska demuxer produces, so stream copying with some generic remuxing example I am really sure that is NOT how the packets come in. > That's a choice Michael have > made, I think. Possibly someone else, I don't know. Anyway, Michael once > told me that he would like to view palettized frames as "self-contained", > where the palette is part of the video data in an atomic way. I'd be fine with that. Except that is simply not the way it is currently as far as I can tell. > Anyway, things work OK right now, and even if the situation is slightly > confusing, it's probably not a good idea to start messing around with things > until a clearly defined way of storing the palette for the muxers and > demuxers has been presented. I can't see HOW they even COULD work right now?! Otherwise I wouldn't really care, but as far as I can tell I don't see how it could work at all but for some lucky exceptions. What cases have we actually tested to work? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 05:45 PM, Reimar Döffinger wrote: I agree that it should be stored in a side data packet myself normally, and that this is a somewhat weird construction. It probably has to do with the nut format originally, which stores raw palettized data after the video data in the packets. Anyway, I have accepted the facts. For the record, the new ff_reshuffle_raw_rgb() function written by Michael in lavf/rawutils.c that aligns strides properly for AVI and QuickTime, will set a CONTAINS_PAL flag if the packet size is larger than the actual video data. He has hardcoded the palette size to 1024 bytes in that file. Mats The nut format stores the PALETTE after the video data in the packets, nothing else :) In any case, on muxing, the packets will have the palette after the video data in the packets, whether it's AVI or QuickTime. Neither avienc.c or movenc.c uses any side data packets for the palette. Michael's intention has been to enable palette switching in the middle of the stream, hence storage of the palette in each packet, and AVI supports it by using the 'xxpc' chunks in the video data. It is also implemented by now. Mats Not that it couldn't be done with side data packets, though. If it doesn't support side data then the muxers are plain broken. The muxers should support side data, I agree on that. The thing is the packets come in to the AVI, QuickTime and nut muxers with the palette data appended to the video data in the packets. That's a choice Michael have made, I think. Possibly someone else, I don't know. Anyway, Michael once told me that he would like to view palettized frames as "self-contained", where the palette is part of the video data in an atomic way. If the nut muxer stores palette by appending it to the frames, then the demuxer should split it out into side data. Note that I am absolutely not a fan of this side data stuff, but since we already decided to do it like that then that's the way we need to go, not randomly doing one way in one place and differently in another, that just makes for unusable API. I agree on that. I'm a bit confused myself about the differing ways of storing the palette... 2) There are file formats that store it that way and we cannot easily split it into side data. Not sure that can really happen. nut stores the palette in every frame in the file, in its current shape. Anyway, things work OK right now, and even if the situation is slightly confusing, it's probably not a good idea to start messing around with things until a clearly defined way of storing the palette for the muxers and demuxers has been presented. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On Sat, Feb 27, 2016 at 04:15:10PM +0100, Mats Peterson wrote: > On 02/27/2016 04:13 PM, Mats Peterson wrote: > >On 02/27/2016 04:08 PM, Mats Peterson wrote: > >>On 02/27/2016 04:07 PM, Mats Peterson wrote: > >>>On 02/27/2016 04:00 PM, Reimar Döffinger wrote: > On Sat, Feb 27, 2016 at 03:57:06PM +0100, Mats Peterson wrote: > >On 02/27/2016 03:37 PM, Mats Peterson wrote: > >> > >> > >> > >>___ > >>ffmpeg-devel mailing list > >>ffmpeg-devel@ffmpeg.org > >>http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > > > >I suppose this is what you mean, Reimar. Treating the palette, if a > >packet > >contains one at the end of the video data, as being AVPALETTE_SIZE > >bytes > >exclusively. > > Well, actually not really. > If the palette is part of input frame it should be sent as side > data. > I am not sure where this variant comes from. > It might be that it should just be written as is. > Or even if the palette needs to be split it might be > necessary to auto-detect the palette size via > packet size - (width*height*bits per pixel)/8. > But as said, I am fairly unclear on what case that > code is supposed to handle. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > >>> > >>>I agree that it should be stored in a side data packet myself normally, > >>>and that this is a somewhat weird construction. It probably has to do > >>>with the nut format originally, which stores raw palettized data after > >>>the video data in the packets. Anyway, I have accepted the facts. For > >>>the record, the new ff_reshuffle_raw_rgb() function written by Michael > >>>in lavf/rawutils.c that aligns strides properly for AVI and QuickTime, > >>>will set a CONTAINS_PAL flag if the packet size is larger than the > >>>actual video data. He has hardcoded the palette size to 1024 bytes in > >>>that file. > >>> > >>>Mats > >>> > >> > >>The nut format stores the PALETTE after the video data in the packets, > >>nothing else :) > >> > > > >In any case, on muxing, the packets will have the palette after the > >video data in the packets, whether it's AVI or QuickTime. Neither > >avienc.c or movenc.c uses any side data packets for the palette. > > > >Michael's intention has been to enable palette switching in the middle > >of the stream, hence storage of the palette in each packet, and AVI > >supports it by using the 'xxpc' chunks in the video data. It is also > >implemented by now. > > > >Mats > > > > Not that it couldn't be done with side data packets, though. If it doesn't support side data then the muxers are plain broken. If the nut muxer stores palette by appending it to the frames, then the demuxer should split it out into side data. Note that I am absolutely not a fan of this side data stuff, but since we already decided to do it like that then that's the way we need to go, not randomly doing one way in one place and differently in another, that just makes for unusable API. The only reasons to support "palette appended to data" are 1) There are some existing users of the FFmpeg API that rely on it. Ideally we should then change it so it works for all muxers, or the other way round warn that this is a deprecated way of doing things. 2) There are file formats that store it that way and we cannot easily split it into side data. Not sure that can really happen. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 04:13 PM, Mats Peterson wrote: On 02/27/2016 04:08 PM, Mats Peterson wrote: On 02/27/2016 04:07 PM, Mats Peterson wrote: On 02/27/2016 04:00 PM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 03:57:06PM +0100, Mats Peterson wrote: On 02/27/2016 03:37 PM, Mats Peterson wrote: ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I suppose this is what you mean, Reimar. Treating the palette, if a packet contains one at the end of the video data, as being AVPALETTE_SIZE bytes exclusively. Well, actually not really. If the palette is part of input frame it should be sent as side data. I am not sure where this variant comes from. It might be that it should just be written as is. Or even if the palette needs to be split it might be necessary to auto-detect the palette size via packet size - (width*height*bits per pixel)/8. But as said, I am fairly unclear on what case that code is supposed to handle. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I agree that it should be stored in a side data packet myself normally, and that this is a somewhat weird construction. It probably has to do with the nut format originally, which stores raw palettized data after the video data in the packets. Anyway, I have accepted the facts. For the record, the new ff_reshuffle_raw_rgb() function written by Michael in lavf/rawutils.c that aligns strides properly for AVI and QuickTime, will set a CONTAINS_PAL flag if the packet size is larger than the actual video data. He has hardcoded the palette size to 1024 bytes in that file. Mats The nut format stores the PALETTE after the video data in the packets, nothing else :) In any case, on muxing, the packets will have the palette after the video data in the packets, whether it's AVI or QuickTime. Neither avienc.c or movenc.c uses any side data packets for the palette. Michael's intention has been to enable palette switching in the middle of the stream, hence storage of the palette in each packet, and AVI supports it by using the 'xxpc' chunks in the video data. It is also implemented by now. Mats Not that it couldn't be done with side data packets, though. -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 04:08 PM, Mats Peterson wrote: On 02/27/2016 04:07 PM, Mats Peterson wrote: On 02/27/2016 04:00 PM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 03:57:06PM +0100, Mats Peterson wrote: On 02/27/2016 03:37 PM, Mats Peterson wrote: ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I suppose this is what you mean, Reimar. Treating the palette, if a packet contains one at the end of the video data, as being AVPALETTE_SIZE bytes exclusively. Well, actually not really. If the palette is part of input frame it should be sent as side data. I am not sure where this variant comes from. It might be that it should just be written as is. Or even if the palette needs to be split it might be necessary to auto-detect the palette size via packet size - (width*height*bits per pixel)/8. But as said, I am fairly unclear on what case that code is supposed to handle. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I agree that it should be stored in a side data packet myself normally, and that this is a somewhat weird construction. It probably has to do with the nut format originally, which stores raw palettized data after the video data in the packets. Anyway, I have accepted the facts. For the record, the new ff_reshuffle_raw_rgb() function written by Michael in lavf/rawutils.c that aligns strides properly for AVI and QuickTime, will set a CONTAINS_PAL flag if the packet size is larger than the actual video data. He has hardcoded the palette size to 1024 bytes in that file. Mats The nut format stores the PALETTE after the video data in the packets, nothing else :) In any case, on muxing, the packets will have the palette after the video data in the packets, whether it's AVI or QuickTime. Neither avienc.c or movenc.c uses any side data packets for the palette. Michael's intention has been to enable palette switching in the middle of the stream, hence storage of the palette in each packet, and AVI supports it by using the 'xxpc' chunks in the video data. It is also implemented by now. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 04:07 PM, Mats Peterson wrote: On 02/27/2016 04:00 PM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 03:57:06PM +0100, Mats Peterson wrote: On 02/27/2016 03:37 PM, Mats Peterson wrote: ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I suppose this is what you mean, Reimar. Treating the palette, if a packet contains one at the end of the video data, as being AVPALETTE_SIZE bytes exclusively. Well, actually not really. If the palette is part of input frame it should be sent as side data. I am not sure where this variant comes from. It might be that it should just be written as is. Or even if the palette needs to be split it might be necessary to auto-detect the palette size via packet size - (width*height*bits per pixel)/8. But as said, I am fairly unclear on what case that code is supposed to handle. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I agree that it should be stored in a side data packet myself normally, and that this is a somewhat weird construction. It probably has to do with the nut format originally, which stores raw palettized data after the video data in the packets. Anyway, I have accepted the facts. For the record, the new ff_reshuffle_raw_rgb() function written by Michael in lavf/rawutils.c that aligns strides properly for AVI and QuickTime, will set a CONTAINS_PAL flag if the packet size is larger than the actual video data. He has hardcoded the palette size to 1024 bytes in that file. Mats The nut format stores the PALETTE after the video data in the packets, nothing else :) -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 04:00 PM, Reimar Döffinger wrote: On Sat, Feb 27, 2016 at 03:57:06PM +0100, Mats Peterson wrote: On 02/27/2016 03:37 PM, Mats Peterson wrote: ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I suppose this is what you mean, Reimar. Treating the palette, if a packet contains one at the end of the video data, as being AVPALETTE_SIZE bytes exclusively. Well, actually not really. If the palette is part of input frame it should be sent as side data. I am not sure where this variant comes from. It might be that it should just be written as is. Or even if the palette needs to be split it might be necessary to auto-detect the palette size via packet size - (width*height*bits per pixel)/8. But as said, I am fairly unclear on what case that code is supposed to handle. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I agree that it should be stored in a side data packet myself normally, and that this is a somewhat weird construction. It probably has to do with the nut format originally, which stores raw palettized data after the video data in the packets. Anyway, I have accepted the facts. For the record, the new ff_reshuffle_raw_rgb() function written by Michael in lavf/rawutils.c that aligns strides properly for AVI and QuickTime, will set a CONTAINS_PAL flag if the packet size is larger than the actual video data. He has hardcoded the palette size to 1024 bytes in that file. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On Sat, Feb 27, 2016 at 03:57:06PM +0100, Mats Peterson wrote: > On 02/27/2016 03:37 PM, Mats Peterson wrote: > > > > > > > >___ > >ffmpeg-devel mailing list > >ffmpeg-devel@ffmpeg.org > >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > I suppose this is what you mean, Reimar. Treating the palette, if a packet > contains one at the end of the video data, as being AVPALETTE_SIZE bytes > exclusively. Well, actually not really. If the palette is part of input frame it should be sent as side data. I am not sure where this variant comes from. It might be that it should just be written as is. Or even if the palette needs to be split it might be necessary to auto-detect the palette size via packet size - (width*height*bits per pixel)/8. But as said, I am fairly unclear on what case that code is supposed to handle. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavf/avienc: Simplify palette handling
On 02/27/2016 03:37 PM, Mats Peterson wrote: ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I suppose this is what you mean, Reimar. Treating the palette, if a packet contains one at the end of the video data, as being AVPALETTE_SIZE bytes exclusively. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
