Re: [FFmpeg-devel] [PATCH v8] lavf: palettized QuickTime video in Matroska
On 12/27/2015 12:11 PM, Mats Peterson wrote: On 12/27/2015 12:10 PM, Hendrik Leppkes wrote: On Sun, Dec 27, 2015 at 11:44 AM, Mats Peterson wrote: Since the ff_get_qtpalette() function has to parse the video sample description in two different ways, i.e. 1) by using the in-memory private data when called from matroskadec.c, and 2) by reading from the file when called from mov.c, it has to use a separate variable for each of these sources. It may seem slightly redundant, but it is unfortunately necessary. I would prefer that nobody touches what I've been doing, since it's working fine right now, and it's very easy to break things if you try to "improve it". Believe me, I've been there. Just to make it perfectly clear that there are no misunderstandings: A shared utility function should offer one clear calling interface, not two just because its convenient. Anything else is a maintenance nightmare. Patch not acceptable unless this is corrected. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel You're not the maintainer, once again. Mats What's more, it doesn't offer "two" interfaces, but it uses two *variables*, one memory (stsd) and one file (pb), of which BOTH are used when called from mov.c, but only one (stsd) when called from matroskadec.c. Surprise surprise. 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 v8] lavf: palettized QuickTime video in Matroska
On 12/27/2015 12:10 PM, Hendrik Leppkes wrote: On Sun, Dec 27, 2015 at 11:44 AM, Mats Peterson wrote: Since the ff_get_qtpalette() function has to parse the video sample description in two different ways, i.e. 1) by using the in-memory private data when called from matroskadec.c, and 2) by reading from the file when called from mov.c, it has to use a separate variable for each of these sources. It may seem slightly redundant, but it is unfortunately necessary. I would prefer that nobody touches what I've been doing, since it's working fine right now, and it's very easy to break things if you try to "improve it". Believe me, I've been there. Just to make it perfectly clear that there are no misunderstandings: A shared utility function should offer one clear calling interface, not two just because its convenient. Anything else is a maintenance nightmare. Patch not acceptable unless this is corrected. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel You're not the maintainer, once again. 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 v8] lavf: palettized QuickTime video in Matroska
On Sun, Dec 27, 2015 at 11:44 AM, Mats Peterson wrote: > > Since the ff_get_qtpalette() function has to parse the video sample > description in two different ways, i.e. 1) by using the in-memory > private data when called from matroskadec.c, and 2) by reading from the > file when called from mov.c, it has to use a separate variable for each > of these sources. It may seem slightly redundant, but it is > unfortunately necessary. I would prefer that nobody touches what I've > been doing, since it's working fine right now, and it's very easy to > break things if you try to "improve it". Believe me, I've been there. > Just to make it perfectly clear that there are no misunderstandings: A shared utility function should offer one clear calling interface, not two just because its convenient. Anything else is a maintenance nightmare. Patch not acceptable unless this is corrected. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
