Re: [FFmpeg-devel] lurking bugs in the mmx-related assembler code (?)
On Sun, Oct 02, 2016 at 10:59:31PM -0400, compn wrote: > vp3 is a decoder written 10+ years ago by a dev who is no longer > active in ffmpeg. > > we have many decoders and encoders (and other code) in ffmpeg that have > not been audited (to my knowledge). > > i know this isnt an excuse for not looking at the code. just letting > you know the history. Sure, I know the code is old (it is even documented as such). > please dont think carl's tone is unpleasant, maybe he is just > incredulous. Thanks for this comment. I am well aware of how hard it is to weight emotion-loaded expressions, especially when someone else looks wrong in one's expected area of competence. > carl has tested and verified thousands of bugs for ffmpeg. carl also > fixes some of them. I was quite aware of Carl's qualities, but they are surely nice to mention. > carl would like to reproduce the bug. reproducing the bug with an easy > command line, on a dev computer usually makes the bug finding and fixing > quicker. Sure. This is though (in my very humble opinion) a special kind of bug, where Carl did not have to chase the "offending code" in the C library (it is not the C library which is at fault) but directly look at libavcodec/x86/vp3dsp.asm and the places where it is used. OTOH everything seems to go very well and I am looking forward to some elegant solution, with confidence. > > To give you an example of successful code auditing, the corresponding > > UB-problem in libtheora was properly fixed without anybody at Xiph > > having to install musl. > > That's why I still believe that auditing the code is more useful than This can look misleading, sorry for that. I meant, auditing particular parts of the code, for the actual, identified problem. > > hunting once again the hard-to-pinpoint symptoms of the already known > > cause. > > do you have any suggestions for how the ffmpeg project could do > successful code audits? I would like to, but regrettably I do not have any bright ideas about this "in general". Auditing is hard. To look at the assembler routines and check for a particular UB, whether the FPU state is restored before calling C library, this has a limited scope and is what I suggested. > i really dont see much enthusiasm for line by line code auditing within > ffmpeg. in fact, i'd say people would rather re-write an entirely new > piece of code than try to clean up the old code. this isnt a knock on > anyone or any piece of code, just my observation. I can only agree with every word. Regards, Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
Le duodi 12 vendémiaire, an CCXXV, wm4 a écrit : > You could mention that this is due to FFmpeg violating the ABI > (apparently). Still, using floats in malloc() looks like a very bad idea, borderline recipe for disaster. Especially on x86_32. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
2016-10-03 3:52 GMT+02:00 Carl Eugen Hoyos : > New patch attached. > +Compilation with @uref{http://www.musl-libc.org/, musl} on x86-64 is supposed > +to work out-of-the-box. Is this true or is it just working by accident? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] lurking bugs in the mmx-related assembler code (?)
Le duodi 12 vendémiaire, an CCXXV, Carl Eugen Hoyos a écrit : > I suspect I found the responsible code: > http://git.musl-libc.org/cgit/musl/tree/src/malloc/malloc.c#n114 Urgh. This is even worse than I imagined. FFmpeg is using undefined behaviours by calling it without resetting the state, but this is also completely undefined behaviour on their side. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libswscale/swscale_unscaled.c: UHD Resolution Performance Increase for Color Space Convertions / NVENC Related
Hello, This patch is done for performance increase on UHD or above resolution color space convertions. Some SDI sources provide yuv422p10 for 10bit source and uyvy422 for 8 bit source. To encode these sources with NVENC 10 bits, there is a need to convert these color spaces to P010. Before patch for UHD and above resolutions, convertion could not exceed ~25-30 fps, which can not be used for a 50 fps encoding. This patch fixes this problem. Also, color space convertion speed for 10bit YUV422P to 8bit YUV420P is having the same problem. If anybody wants to encode 10 bits source in 8 bits for UHD or above resolutions could not achive high frame rate ratio as well. This patch also fixes this problem. I think it will be good to apply this patch to avoid performance loss in high resolutions. Kind Regards, 0001-For-10bit-SDI-Sources-to-10bit-NVENC-Encode.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libswscale/swscale_unscaled.c: UHD Resolution Performance Increase for Color Space Convertions / NVENC Related
2016-10-03 11:35 GMT+02:00 Ali KIZIL : > +/* yuv422p10_to_yuv420p */ > +if ((srcFormat == AV_PIX_FMT_YUV422P10 || srcFormat == > AV_PIX_FMT_YUVA422P10) && > +(dstFormat == AV_PIX_FMT_YUV420P) || srcFormat == > AV_PIX_FMT_YUVA420P) > +c->swscale = yuv422p10ToYuv420pWrapper; I believe there is something wrong. The actual wrapper function is missing alpha handling. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libswscale/swscale_unscaled.c: UHD Resolution Performance Increase for Color Space Convertions / NVENC Related
On Mon, Oct 3, 2016 at 11:35 AM, Ali KIZIL wrote: > Hello, > > This patch is done for performance increase on UHD or above resolution > color space convertions. > Some SDI sources provide yuv422p10 for 10bit source and uyvy422 for 8 bit > source. > To encode these sources with NVENC 10 bits, there is a need to convert > these color spaces to P010. > > Before patch for UHD and above resolutions, convertion could not exceed > ~25-30 fps, which can not be used for a 50 fps encoding. > This patch fixes this problem. > > Also, color space convertion speed for 10bit YUV422P to 8bit YUV420P is > having the same problem. > If anybody wants to encode 10 bits source in 8 bits for UHD or above > resolutions could not achive high frame rate ratio as well. > This patch also fixes this problem. > > I think it will be good to apply this patch to avoid performance loss in > high resolutions. > These conversion functions butcher quality by using only every second row of chroma for 422 -> 420 conversions without any interpolation, and they don't dither for 10->8 bit conversions. Not sure its a good idea to do that, even more so without an option to defeat these low-quality variants with the swscale flags. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
On Mon, Oct 3, 2016 at 2:36 AM, Ronald S. Bultje wrote: > Hi, > > On Sun, Oct 2, 2016 at 7:51 PM, Carl Eugen Hoyos wrote: > >> Hi! >> >> Attached patch adds a musl section to doc/platform. > > > This is counter-productive... Let's work with the musl-devs and fix their > libc-alternative? > I agree that adding such a note is not really going to help anything, if anything its going to cause more sillyness. Lets just fix the bugs, and not call something simply unsupported. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
2016-10-03 12:30 GMT+02:00 Hendrik Leppkes : > Lets just fix the bugs Sorry: Which bugs exactly should we just fix? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] lurking bugs in the mmx-related assembler code (?)
> > http://git.musl-libc.org/cgit/musl/tree/src/malloc/malloc.c#n114 > > Urgh. This is even worse than I imagined. FFmpeg is using undefined > behaviours by calling it without resetting the state, but this is also > completely undefined behaviour on their side. I feel a duty to remind, in the most positive and friendly tone: The author of the referred code acts in his actual area of competence (C libraries and standards). The comments here on the C library code and standard compliance come from developers having a different competence area (multimedia programming). As bright as the people here are, they land in a foreign area, which accidentally leads to statements like the above. Regards, Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
On Mon, Oct 03, 2016 at 12:37:29PM +0200, Carl Eugen Hoyos wrote: > 2016-10-03 12:30 GMT+02:00 Hendrik Leppkes : > > > Lets just fix the bugs > > Sorry: Which bugs exactly should we just fix? > > Carl Eugen Please fix the UB in the MMX-code. Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libswscale/swscale_unscaled.c: UHD Resolution Performance Increase for Color Space Convertions / NVENC Related
2016-10-03 13:28 GMT+03:00 Hendrik Leppkes : > On Mon, Oct 3, 2016 at 11:35 AM, Ali KIZIL wrote: > > Hello, > > > > This patch is done for performance increase on UHD or above resolution > > color space convertions. > > Some SDI sources provide yuv422p10 for 10bit source and uyvy422 for 8 bit > > source. > > To encode these sources with NVENC 10 bits, there is a need to convert > > these color spaces to P010. > > > > Before patch for UHD and above resolutions, convertion could not exceed > > ~25-30 fps, which can not be used for a 50 fps encoding. > > This patch fixes this problem. > > > > Also, color space convertion speed for 10bit YUV422P to 8bit YUV420P is > > having the same problem. > > If anybody wants to encode 10 bits source in 8 bits for UHD or above > > resolutions could not achive high frame rate ratio as well. > > This patch also fixes this problem. > > > > I think it will be good to apply this patch to avoid performance loss in > > high resolutions. > > > > These conversion functions butcher quality by using only every second > row of chroma for 422 -> 420 conversions without any interpolation, > and they don't dither for 10->8 bit conversions. > Not sure its a good idea to do that, even more so without an option to > defeat these low-quality variants with the swscale flags. > > - Hendrik > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Hello Hendrik, Yes, only every second row of chroma for 422 -> 420 conversions without any interpolation is used. Where I can find an interpolation example ? Also, the code is done for performance criteria for UHD and above resolution in real time encoding. In myside test, I didnt notice a gap. For sure, I am not a video quality expert. Can you tell me how to compare quality ? Can you explain more for "they don't dither for 10->8 bit conversions." I couldnt understand this. Kind Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libswscale/swscale_unscaled.c: UHD Resolution Performance Increase for Color Space Convertions / NVENC Related
2016-10-03 13:06 GMT+03:00 Carl Eugen Hoyos : > 2016-10-03 11:35 GMT+02:00 Ali KIZIL : > > > +/* yuv422p10_to_yuv420p */ > > +if ((srcFormat == AV_PIX_FMT_YUV422P10 || srcFormat == > AV_PIX_FMT_YUVA422P10) && > > +(dstFormat == AV_PIX_FMT_YUV420P) || srcFormat == > AV_PIX_FMT_YUVA420P) > > +c->swscale = yuv422p10ToYuv420pWrapper; > > I believe there is something wrong. > > The actual wrapper function is missing alpha handling. > > Carl Eugen > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Hello Carl, Yes, Alpha channel is not managed. So it should be; > +/* yuv422p10_to_yuv420p */ > +if ((srcFormat == AV_PIX_FMT_YUV422P10) && > +(dstFormat == AV_PIX_FMT_YUV420P)) > +c->swscale = yuv422p10ToYuv420pWrapper; I will get it corrected after collecting all comments. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libswscale/swscale_unscaled.c: UHD Resolution Performance Increase for Color Space Convertions / NVENC Related
2016-10-03 13:28 GMT+03:00 Hendrik Leppkes : > On Mon, Oct 3, 2016 at 11:35 AM, Ali KIZIL wrote: > > Hello, > > > > This patch is done for performance increase on UHD or above resolution > > color space convertions. > > Some SDI sources provide yuv422p10 for 10bit source and uyvy422 for 8 bit > > source. > > To encode these sources with NVENC 10 bits, there is a need to convert > > these color spaces to P010. > > > > Before patch for UHD and above resolutions, convertion could not exceed > > ~25-30 fps, which can not be used for a 50 fps encoding. > > This patch fixes this problem. > > > > Also, color space convertion speed for 10bit YUV422P to 8bit YUV420P is > > having the same problem. > > If anybody wants to encode 10 bits source in 8 bits for UHD or above > > resolutions could not achive high frame rate ratio as well. > > This patch also fixes this problem. > > > > I think it will be good to apply this patch to avoid performance loss in > > high resolutions. > > > > These conversion functions butcher quality by using only every second > row of chroma for 422 -> 420 conversions without any interpolation, > and they don't dither for 10->8 bit conversions. > Not sure its a good idea to do that, even more so without an option to > defeat these low-quality variants with the swscale flags. > > - Hendrik > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Hello Hendrik, Also, Can I assume that the problem is for 10 bit to 8 bit convertion ? Because, other 2 functions do 10bit to 10bit or 8bit to 10 bit operation. I can dividepatch to 3 sub-patches as well. Kind Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libswscale/swscale_unscaled.c: UHD Resolution Performance Increase for Color Space Convertions / NVENC Related
2016-10-03 12:48 GMT+02:00 Ali KIZIL : > Yes, Alpha channel is not managed. > > So it should be; > >> +/* yuv422p10_to_yuv420p */ >> +if ((srcFormat == AV_PIX_FMT_YUV422P10) && >> +(dstFormat == AV_PIX_FMT_YUV420P)) >> +c->swscale = yuv422p10ToYuv420pWrapper; (Apart from "too many parenthesis") I would prefer if alpha gets handled. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libswscale/swscale_unscaled.c: UHD Resolution Performance Increase for Color Space Convertions / NVENC Related
2016-10-03 14:09 GMT+03:00 Carl Eugen Hoyos : > 2016-10-03 12:48 GMT+02:00 Ali KIZIL : > > > Yes, Alpha channel is not managed. > > > > So it should be; > > > >> +/* yuv422p10_to_yuv420p */ > >> +if ((srcFormat == AV_PIX_FMT_YUV422P10) && > >> +(dstFormat == AV_PIX_FMT_YUV420P)) > >> +c->swscale = yuv422p10ToYuv420pWrapper; > > (Apart from "too many parenthesis") > I would prefer if alpha gets handled. > > Carl Eugen > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Hello Carl, Got it. I dont have any source with alpha. Even I do blind coding, I am not sure if it will be working OK. So, maybe it will be a better idea that if someone improve code for alpha channel. Kind Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
Hello, On Mon, Oct 3, 2016 at 11:38 AM, Carl Eugen Hoyos wrote: > 2016-10-03 3:52 GMT+02:00 Carl Eugen Hoyos : > >> New patch attached. > >> +Compilation with @uref{http://www.musl-libc.org/, musl} on x86-64 is >> supposed >> +to work out-of-the-box. > > Is this true or is it just working by accident? AMD64 dictates that float operations must occur with scalar SSE operations, so it's not working by accident. It works because float operations don't use x87 stack (which is shared with MMX) Best regards, Guillaume -- Wearing a Rolex is like driving an Audi: It says you've got some money, but nothing to say. John Lefèvre ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
Hi! 2016-10-03 13:05 GMT+02:00 Guillaume POIRIER : > Hello, > > On Mon, Oct 3, 2016 at 11:38 AM, Carl Eugen Hoyos wrote: >> 2016-10-03 3:52 GMT+02:00 Carl Eugen Hoyos : >> >>> New patch attached. >> >>> +Compilation with @uref{http://www.musl-libc.org/, musl} on x86-64 is >>> supposed >>> +to work out-of-the-box. >> >> Is this true or is it just working by accident? > > AMD64 dictates that float operations must occur with scalar SSE > operations, so it's not working by accident. It works because float > operations don't use x87 stack (which is shared with MMX) Thank you for the explanation! Ping on the patch: We will not guard every asm call with emms_c() and Rich will not change his alloc() implementation for us: Can't we just document that it doesn't work and return to bugs that we can fix? Or should we consider to guard the three calls to mallc(), realloc() and free()? This works here as expected with 32bit musl but likely does not fix the underlying issue. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libswscale/swscale_unscaled.c: UHD Resolution Performance Increase for Color Space Convertions / NVENC Related
2016-10-03 13:11 GMT+02:00 Ali KIZIL : > 2016-10-03 14:09 GMT+03:00 Carl Eugen Hoyos : > >> 2016-10-03 12:48 GMT+02:00 Ali KIZIL : >> >> > Yes, Alpha channel is not managed. >> > >> > So it should be; >> > >> >> +/* yuv422p10_to_yuv420p */ >> >> +if ((srcFormat == AV_PIX_FMT_YUV422P10) && >> >> +(dstFormat == AV_PIX_FMT_YUV420P)) >> >> +c->swscale = yuv422p10ToYuv420pWrapper; >> >> (Apart from "too many parenthesis") >> I would prefer if alpha gets handled. > Got it. I dont have any source with alpha. This is about conversion of frames without alpha into frames with an alpha channel, so I don't think you need any source with alpha. Just set the opacity to 0xFF. I suspect you should pay attention to the dithering for the moment though. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libswscale/swscale_unscaled.c: UHD Resolution Performance Increase for Color Space Convertions / NVENC Related
2016-10-03 14:39 GMT+03:00 Carl Eugen Hoyos : > 2016-10-03 13:11 GMT+02:00 Ali KIZIL : > > 2016-10-03 14:09 GMT+03:00 Carl Eugen Hoyos : > > > >> 2016-10-03 12:48 GMT+02:00 Ali KIZIL : > >> > >> > Yes, Alpha channel is not managed. > >> > > >> > So it should be; > >> > > >> >> +/* yuv422p10_to_yuv420p */ > >> >> +if ((srcFormat == AV_PIX_FMT_YUV422P10) && > >> >> +(dstFormat == AV_PIX_FMT_YUV420P)) > >> >> +c->swscale = yuv422p10ToYuv420pWrapper; > >> > >> (Apart from "too many parenthesis") > >> I would prefer if alpha gets handled. > > > Got it. I dont have any source with alpha. > > This is about conversion of frames without alpha into > frames with an alpha channel, so I don't think you need > any source with alpha. > Just set the opacity to 0xFF. > > I suspect you should pay attention to the dithering for > the moment though. > > Carl Eugen > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Hello Carl, OK, Well noted the info about 0xFF. I will do it. You and Hendrik are right. I googled around and understood what you mean by dithering. This will happen for function 10bit to 8bit convertion yuv422p10ToYuv420pWrapper. Rest 2 functions are 10bit to 10bit and 8 bit to 10bit, so they do not have dithering problem. I need to check how to do interpolation. Any example will be helpful. Kind Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
Hi, On Mon, Oct 3, 2016 at 7:37 AM, Carl Eugen Hoyos wrote: > Hi! > > 2016-10-03 13:05 GMT+02:00 Guillaume POIRIER : > > Hello, > > > > On Mon, Oct 3, 2016 at 11:38 AM, Carl Eugen Hoyos > wrote: > >> 2016-10-03 3:52 GMT+02:00 Carl Eugen Hoyos : > >> > >>> New patch attached. > >> > >>> +Compilation with @uref{http://www.musl-libc.org/, musl} on x86-64 is > supposed > >>> +to work out-of-the-box. > >> > >> Is this true or is it just working by accident? > > > > AMD64 dictates that float operations must occur with scalar SSE > > operations, so it's not working by accident. It works because float > > operations don't use x87 stack (which is shared with MMX) > > Thank you for the explanation! > > Ping on the patch: The patch is unlikely to assist in fixing the issue and is likely to cause further inflammation. Therefore I would prefer you did not apply the patch and also please don't send a new version that is differently worded. I would prefer to work with upstream (musl) and fix the issue where ffmpeg running with musl crashes. Whether that means changing ffmpeg or musl remains to be seen. Let's chat with the developers and figure something out. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] lurking bugs in the mmx-related assembler code (?)
Hi, On Sun, Oct 2, 2016 at 11:04 PM, compn wrote: > On Sun, 2 Oct 2016 08:29:22 -0400 > "Ronald S. Bultje" wrote: > > > I also think we could contact musl developers and see what's going on > > there. We certainly shouldn't blindly fix this bug by adding an emms > > in a random place, to me that's like opening pandora's box. > > on irc i pointed dalias to this thread. > > or you can just ask on #musl in freenode. Thanks, that's very helpful, I'll chat with them today. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
On Mon, Oct 3, 2016 at 1:37 PM, Carl Eugen Hoyos wrote: > Hi! > > 2016-10-03 13:05 GMT+02:00 Guillaume POIRIER : >> Hello, >> >> On Mon, Oct 3, 2016 at 11:38 AM, Carl Eugen Hoyos wrote: >>> 2016-10-03 3:52 GMT+02:00 Carl Eugen Hoyos : >>> New patch attached. >>> +Compilation with @uref{http://www.musl-libc.org/, musl} on x86-64 is supposed +to work out-of-the-box. >>> >>> Is this true or is it just working by accident? >> >> AMD64 dictates that float operations must occur with scalar SSE >> operations, so it's not working by accident. It works because float >> operations don't use x87 stack (which is shared with MMX) > > Thank you for the explanation! > > Ping on the patch: > We will not guard every asm call with emms_c() and Rich will not > change his alloc() implementation for us: Can't we just document > that it doesn't work and return to bugs that we can fix? > > Or should we consider to guard the three calls to mallc(), realloc() > and free()? > This works here as expected with 32bit musl but likely does not > fix the underlying issue. > There is no reason to call anything unsupported because FFmpeg violates the C calling convention, nor is there a reason to add overhead to every single call. The underlying problem is that mmx code is mixed with allocations, which seems like an unusual case to begin with since allocations should not be part of performance critical loops or something like that. So, the appropriate answer is to find the code in question that causes these issues, and just add an emms there, or re-structure it to take the allocations out of the mmx code. Anything else just masks potential issues. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/mediacodecdec_h2645: fix nalu data_size type
On Sat, Oct 01, 2016 at 12:52:53PM +0200, Matthieu Bouron wrote: > On Fri, Sep 30, 2016 at 07:01:09PM +0200, wm4 wrote: > > On Fri, 30 Sep 2016 17:51:42 +0100 > > Josh de Kock wrote: > > > > > On 30/09/2016 17:34, Matthieu Bouron wrote: > > > > From: Matthieu Bouron > > > > > > > > --- > > > > libavcodec/mediacodecdec_h2645.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/libavcodec/mediacodecdec_h2645.c > > > > b/libavcodec/mediacodecdec_h2645.c > > > > index 122be88..9f8110c 100644 > > > > --- a/libavcodec/mediacodecdec_h2645.c > > > > +++ b/libavcodec/mediacodecdec_h2645.c > > > > @@ -154,7 +154,7 @@ static int h264_set_extradata(AVCodecContext > > > > *avctx, FFAMediaFormat *format) > > > > > > > > if (pps && sps) { > > > > uint8_t *data = NULL; > > > > -size_t data_size = 0; > > > > +int data_size = 0; > > > > > > > > if ((ret = h2645_ps_to_nalu(sps->data, sps->data_size, &data, > > > > &data_size)) < 0) { > > > > goto done; > > > > > > > > > > Shouldn't you change h2645_ps_to_nalu()'s size arguments to size_t as > > > well? > > > > FFmpeg uses int instead of size_t in a LOT of places (including API). > > Might be a little bit too late to fix this convention. > > Yes. On the other hand: > * PPS.data_size type is size_t and PPS.data is uint8_t[4096]. > * ff_AMediaFormat_SetBuffer takes a size_t for the data_size argument. > > IMHO, I don't think switching to size_t is a real improvement here but if > you think it is I can update the patch (otherwise I'll wait another day > and push it as is). Pushed. Matthieu ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
2016-10-03 13:57 GMT+02:00 Hendrik Leppkes : > The underlying problem is that mmx code is mixed with allocations, Definitely. > which seems like an unusual case to begin with I am not sure if I understand this but one instance is calling radix_sort() in the dnxhd encoder. Below is what is needed to run the dnxhd fate tests here (the hr hq test crashes), the second hunk is needed to fix thread joining and avoid an endless loop. There are also issues with h26x. You may be interested in this post that contains a link to a work-around in musl: http://www.openwall.com/lists/musl/2016/09/14/8 No more comments from me, Carl Eugen diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c index 88edd6b..5b4a36a 100644 --- a/libavcodec/dnxhdenc.c +++ b/libavcodec/dnxhdenc.c @@ -1117,6 +1117,7 @@ static int dnxhd_encode_fast(AVCodecContext *avctx, DNXHDEncContext *ctx) if (RC_VARIANCE) avctx->execute2(avctx, dnxhd_mb_var_thread, NULL, NULL, ctx->m.mb_height); +emms_c(); radix_sort(ctx->mb_cmp, ctx->m.mb_num); for (x = 0; x < ctx->m.mb_num && max_bits > ctx->frame_bits; x++) { int mb = ctx->mb_cmp[x].mb; @@ -1214,6 +1215,7 @@ FF_ENABLE_DEPRECATION_WARNINGS ff_side_data_set_encoder_stats(pkt, ctx->qscale * FF_QP2LAMBDA, NULL, 0, AV_PICTURE_TYPE_I); +emms_c(); pkt->flags |= AV_PKT_FLAG_KEY; *got_packet = 1; return 0; ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
Hi, On Mon, Oct 3, 2016 at 8:46 AM, Carl Eugen Hoyos wrote: > 2016-10-03 13:57 GMT+02:00 Hendrik Leppkes : > > > The underlying problem is that mmx code is mixed with allocations, > > Definitely. > > > which seems like an unusual case to begin with > > I am not sure if I understand this but one instance is calling radix_sort() > in the dnxhd encoder. > > Below is what is needed to run the dnxhd fate tests here (the hr hq test > crashes), the second hunk is needed to fix thread joining and avoid an > endless loop. There are also issues with h26x. > > You may be interested in this post that contains a link to a work-around > in musl: http://www.openwall.com/lists/musl/2016/09/14/8 > > No more comments from me, Carl Eugen > > diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c > index 88edd6b..5b4a36a 100644 > --- a/libavcodec/dnxhdenc.c > +++ b/libavcodec/dnxhdenc.c > @@ -1117,6 +1117,7 @@ static int dnxhd_encode_fast(AVCodecContext > *avctx, DNXHDEncContext *ctx) > if (RC_VARIANCE) > avctx->execute2(avctx, dnxhd_mb_var_thread, > NULL, NULL, ctx->m.mb_height); > +emms_c(); > radix_sort(ctx->mb_cmp, ctx->m.mb_num); > for (x = 0; x < ctx->m.mb_num && max_bits > ctx->frame_bits; x++) > { > int mb = ctx->mb_cmp[x].mb; > @@ -1214,6 +1215,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > > ff_side_data_set_encoder_stats(pkt, ctx->qscale * FF_QP2LAMBDA, > NULL, 0, AV_PICTURE_TYPE_I); > > +emms_c(); > pkt->flags |= AV_PKT_FLAG_KEY; > *got_packet = 1; > return 0; I don't think we should litter each and every decoder/encoder with calls to emms_c(). Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avcodec/cuvid: set best_effort_timestamp instead of frame pts
> Please just leave the pts stuff as-is, or we'll have to undo these > changes right in the next merge again, which deprecates pkt_pts and > favors using AVFrame.pts for decoding as well (less redundant fields). > Setting both for the time being should not have any negative effects. Ok, I'll leave that one out. Only thing that needs changing with those merges would be deprecation guards around setting pkt_pts then. > Settings best effort in addition seems slightly weird, do any other > decoders actually set that by themself? No, but no other decoder implements the new decode API yet, which doesn't do so for you anymore. Merging the "lavc: set best effort timestamp if unset when using new decode API" should work just fine as well though. > - Hendrik > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libswscale/swscale_unscaled.c: UHD Resolution Performance Increase for Color Space Convertions / NVENC Related
On Mon, Oct 03, 2016 at 12:35:51PM +0300, Ali KIZIL wrote: > Hello, > > This patch is done for performance increase on UHD or above resolution > color space convertions. > Some SDI sources provide yuv422p10 for 10bit source and uyvy422 for 8 bit > source. > To encode these sources with NVENC 10 bits, there is a need to convert > these color spaces to P010. > > Before patch for UHD and above resolutions, convertion could not exceed > ~25-30 fps, which can not be used for a 50 fps encoding. > This patch fixes this problem. > > Also, color space convertion speed for 10bit YUV422P to 8bit YUV420P is > having the same problem. > If anybody wants to encode 10 bits source in 8 bits for UHD or above > resolutions could not achive high frame rate ratio as well. > This patch also fixes this problem. > > I think it will be good to apply this patch to avoid performance loss in > high resolutions. > > Kind Regards, > swscale_unscaled.c | 140 > + > 1 file changed, 140 insertions(+) > 5c347db66a9a32fbb91cd17ea768143bc768f948 > 0001-For-10bit-SDI-Sources-to-10bit-NVENC-Encode.patch > From 9a680c588248abec30f7a4afdcf7a18c58766ade Mon Sep 17 00:00:00 2001 > From: Sayit BELET > Date: Mon, 3 Oct 2016 12:09:58 +0300 > Subject: [PATCH] For 10bit SDI Sources to 10bit NVENC Encode 10bit YUV422P to > P010 yuv422p10ToP010Wrapper breaks fate --- ./tests/ref/vsynth/vsynth1-vc2-422p10 2016-10-02 14:30:45.987076676 +0200 +++ tests/data/fate/vsynth1-vc2-422p10 2016-10-03 15:45:54.060991862 +0200 @@ -1,4 +1,4 @@ 88e3488e4689cf06e75959c71e2f9d96 *tests/data/fate/vsynth1-vc2-422p10.mov 1684055 tests/data/fate/vsynth1-vc2-422p10.mov -f35dd1c1df4726bb1d75d95e321b0698 *tests/data/fate/vsynth1-vc2-422p10.out.rawvideo -stddev:1.88 PSNR: 42.61 MAXDIFF: 23 bytes: 7603200/ 760320 +3ed919ead3917805fcde08467b6c4907 *tests/data/fate/vsynth1-vc2-422p10.out.rawvideo +stddev:3.56 PSNR: 37.09 MAXDIFF: 38 bytes: 7603200/ 760320 Test vsynth1-vc2-422p10 failed. Look at tests/data/fate/vsynth1-vc2-422p10.err for details. make: *** [fate-vsynth1-vc2-422p10] Error 1 make: *** Waiting for unfinished jobs [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are best at talking, realize last or never when they are wrong. 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/3] avutil/hwcontext_cuda: align allocated frames
applied ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
On Mon, Oct 03, 2016 at 08:01:05AM -0400, Ronald S. Bultje wrote: > > Ping on the patch: > The patch is unlikely to assist in fixing the issue and is likely to cause > further inflammation. Therefore I would prefer you did not apply the patch > and also please don't send a new version that is differently worded. > > I would prefer to work with upstream (musl) and fix the issue where ffmpeg > running with musl crashes. Whether that means changing ffmpeg or musl > remains to be seen. Let's chat with the developers and figure something out. The standard C library API is what it is called, a standard. What you are talking about is to ask Rich to modify musl to hide ffmpeg's non-compliance bug (which glibc/uclibc do by sheer luck but this may change any time). With all the competence present here, is it really infeasible to improve the code structure so that it does not involve the C library in the bit-crunching performance critical paths?? Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
Hi Rune, On Mon, Oct 3, 2016 at 10:26 AM, wrote: > On Mon, Oct 03, 2016 at 08:01:05AM -0400, Ronald S. Bultje wrote: > > > Ping on the patch: > > > The patch is unlikely to assist in fixing the issue and is likely to > cause > > further inflammation. Therefore I would prefer you did not apply the > patch > > and also please don't send a new version that is differently worded. > > > > I would prefer to work with upstream (musl) and fix the issue where > ffmpeg > > running with musl crashes. Whether that means changing ffmpeg or musl > > remains to be seen. Let's chat with the developers and figure something > out. > > The standard C library API is what it is called, a standard. > > What you are talking about is to ask Rich to modify musl to hide ffmpeg's > non-compliance bug (which glibc/uclibc do by sheer luck but this may change > any time). > > With all the competence present here, is it really infeasible to improve > the code structure so that it does not involve the C library in the > bit-crunching performance critical paths?? I'm sure you understand that screaming around like a madman is unlikely to improve anything. - I don't want to litter the code with emms calls all around calls to libc functions, certainly not every en/decoder, this would have to be in generic code only; - calling emms_c before calling user callbacks or malloc/free calls is potentially doable, but doesn't make us abide to the standard in a literal sense. We also need to go over the code and see how many changes this requires; - or we can just do what we do now and work with musl people to change their code. If it turns out the first two are undoable and musl devs are unwilling to do this, then we'll have to reconsider Carl's patch and call musl on x86-32 unsupported, with a link to the relevant discussion to back up our reasoning (to prevent bystanders from calling us trolls). Again, this requires some time and thought. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
On Mon, Oct 03, 2016 at 04:26:50PM +0200, u-9...@aetey.se wrote: > With all the competence present here, is it really infeasible to improve > the code structure so that it does not involve the C library in the > bit-crunching performance critical paths?? Bad wording. Should be: "assembler-implemented" paths. There is nothing wrong with using C library's routines - they are well optimized. But if they are relied upon, they have to be respected, they assume namely that the caller behaves according to the contract. If you have a real need to do things which are incompatible with the API contract, then the C library simply is not available. Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] libavcodec/ivi_dsp.c: fix warning due to misleading indentation
On 02/10/2016 19:46, Adriano Pallavicino wrote: LGTM. Will apply both patches squashed in a day if there are no further comments. -- Josh ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavformat/bink.c: fix warning due to misleading indentation
On 02/10/2016 19:35, Josh de Kock wrote: Hi Adriano, We appreciate the patches, but is it possible you could maybe collate your cosmetic patches and send them as a larger set? This patch LGTM though, will push tomorrow if no further comments. Applied. -- Josh ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
Ronald, I sincerely appreciate that you are taking the effort to talk to me and explain your position. Unfortunately in your messages you consistently ignored a crucial point and this is the last time I try to retell it: > On Mon, Oct 3, 2016 at 10:26 AM, wrote: > > What you are talking about is to ask Rich to modify musl to hide ffmpeg's > > non-compliance bug (which glibc/uclibc do by sheer luck but this may change > > any time). Which you answer with > - or we can just do what we do now and work with musl people to change > their code. Once again, musl has absolutely no reason to change its code, their code is well done and fully compliant. Their responsibility is rather to the contrary, _not_ to make changes which would hide sneaky bugs in applications. UB is a quite hideous bug. > If it turns out the first two are undoable and musl devs are > unwilling to do this, then we'll have to reconsider Carl's patch and call > musl on x86-32 unsupported, with a link to the relevant discussion to back You can not just "call musl on x86-32 unsupported", the only honest option is to document that ffmpeg is not compliant/safe. The problem is not in musl. Last time in this discussion let me try to make this fact understood: Musl merely showed you the problem and now you are suggesting to "document that the messenger with his bad news about our health is no longer welcome". > Again, this requires some time and thought. This sounds very reasonable. Best regards, Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
On Mon, 3 Oct 2016 17:01:12 +0200 u-9...@aetey.se wrote: > Ronald, > > I sincerely appreciate that you are taking the effort to talk to me > and explain your position. > > Unfortunately in your messages you consistently ignored a crucial point > and this is the last time I try to retell it: > > > On Mon, Oct 3, 2016 at 10:26 AM, wrote: > > > What you are talking about is to ask Rich to modify musl to hide ffmpeg's > > > non-compliance bug (which glibc/uclibc do by sheer luck but this may > > > change > > > any time). > > Which you answer with > > > - or we can just do what we do now and work with musl people to change > > their code. > > Once again, musl has absolutely no reason to change its code, > their code is well done and fully compliant. > > Their responsibility is rather to the contrary, _not_ to make changes > which would hide sneaky bugs in applications. UB is a quite hideous bug. > > > If it turns out the first two are undoable and musl devs are > > unwilling to do this, then we'll have to reconsider Carl's patch and call > > musl on x86-32 unsupported, with a link to the relevant discussion to back > > You can not just "call musl on x86-32 unsupported", the only honest option > is to document that ffmpeg is not compliant/safe. > > The problem is not in musl. > Last time in this discussion let me try to make this fact understood: > > Musl merely showed you the problem and now you are suggesting to "document > that the messenger with his bad news about our health is no longer welcome". musl could also choose to abandon its incredibly "clever" hack (that makes almost everyone who sees it go "WTF"), and, as was pointed our on IRC, probably increase the efficiency and readability of the code in question. Yes, musl is technically in the right, but only technically. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] avformat/matroska: write FlagInterlaced element in WebM
Hi, > On Oct 2, 2016, at 7:14 PM, James Almer wrote: > > On 9/27/2016 3:03 PM, James Almer wrote: >> It's listed as supported in both https://www.webmproject.org/docs/container/ >> and https://matroska.org/technical/specs/index.html >> >> Signed-off-by: James Almer >> --- >> libavformat/matroskaenc.c | 41 + >> 1 file changed, 21 insertions(+), 20 deletions(-) > > Ping. Untested but from a read through, the patch looks good. FlagInterlaced is supported in both Matroska and webM, whereas FieldOrder (a new field from the CELLAR work on Matroska) is only supported in Matroska. IMHO though I think that FlagInterlaced without information on FieldOrder is not so useful and that the webM project should consider adopting FieldOrder as well. I'd prefer to see a patch to webM to consider adding FieldOrder to the format rather than see FieldOrder removed from the webM muxer in FFmpeg. Dave Rice ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] avformat/matroska: write FlagInterlaced element in WebM
On 10/3/2016 12:27 PM, Dave Rice wrote: > Hi, > >> On Oct 2, 2016, at 7:14 PM, James Almer wrote: >> >> On 9/27/2016 3:03 PM, James Almer wrote: >>> It's listed as supported in both https://www.webmproject.org/docs/container/ >>> and https://matroska.org/technical/specs/index.html >>> >>> Signed-off-by: James Almer >>> --- >>> libavformat/matroskaenc.c | 41 + >>> 1 file changed, 21 insertions(+), 20 deletions(-) >> >> Ping. > > Untested but from a read through, the patch looks good. FlagInterlaced is > supported in both Matroska and webM, whereas FieldOrder (a new field from the > CELLAR work on Matroska) is only supported in Matroska. IMHO though I think > that FlagInterlaced without information on FieldOrder is not so useful and > that the webM project should consider adopting FieldOrder as well. I'd prefer > to see a patch to webM to consider adding FieldOrder to the format rather > than see FieldOrder removed from the webM muxer in FFmpeg. FieldOrder is not being removed from the WebM muxer with this patch. It, alongside FlagInterlaced, was never written by it. Only by the Matroska muxer. This patch follows the current spec and adds FlagInterlaced to WebM by moving the mode check to only filter out FieldOrder instead of the two elements when targeting WebM. If Google ever changes the spec to also support FieldOrder then adding it will be as simple as removing the mode check. > > Dave Rice > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavformat/bink.c: fix warning due to misleading indentation
Sure. 2016-10-03 16:48 GMT+02:00 Josh de Kock : > On 02/10/2016 19:35, Josh de Kock wrote: > >> Hi Adriano, >> >> We appreciate the patches, but is it possible you could maybe collate >> your cosmetic patches and send them as a larger set? >> >> This patch LGTM though, will push tomorrow if no further comments. >> >> > Applied. > > > -- > Josh > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] lurking bugs in the mmx-related assembler code (?)
On Mon, 3 Oct 2016 12:45:13 +0200 u-9...@aetey.se wrote: > > > http://git.musl-libc.org/cgit/musl/tree/src/malloc/malloc.c#n114 > > > > Urgh. This is even worse than I imagined. FFmpeg is using undefined > > behaviours by calling it without resetting the state, but this is > > also completely undefined behaviour on their side. > > I feel a duty to remind, in the most positive and friendly tone: > > The author of the referred code acts in his actual area of competence > (C libraries and standards). > > The comments here on the C library code and standard compliance come > from developers having a different competence area (multimedia > programming). > > As bright as the people here are, they land in a foreign area, which > accidentally leads to statements like the above. ehe, well.. do your homework before assuming things. ;) rich felker , who wrote musl, was an mplayer and ffmpeg? developer actually. he wrote musl because everyone hated glibc. i contacted both him and even mike melanson (vp3 decoder author) mikes reply: >* I didn't write any VP3 MMX (or other SIMD), so I can't be of much > immediate help, even if I did have perfect recall of the code. > However, I invite you to run 'git blame' on the vp3 code and see how > many of my non-comment lines still persist. rich felker musl reply: > [23:46] yes they're violating the x86 abi > [23:46] at function call time the x87 floating point stack > has to be empty (and thus in x87 mode, not mmx mode) > [23:47] you can't call external functions while in mmx state i dont doubt its a bug in vp3 that needs to be rewritten. but we also go to extreme lengths to blame others... ;D musl could handle it better sure, but should they really write a full c-checker into their lib? would make it as bloated as glibc! -compn ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
On Mon, Oct 03, 2016 at 05:27:19PM +0200, wm4 wrote: > > Musl merely showed you the problem and now you are suggesting to "document > > that the messenger with his bad news about our health is no longer welcome". > > musl could also choose to abandon its incredibly "clever" hack (that > makes almost everyone who sees it go "WTF"), and, as was pointed our on :-D IOW it exposes the _bugs_ in how people are accustomed to reason. > IRC, probably increase the efficiency and readability of the code in > question. Yes, musl is technically in the right, but only technically. Is your opinion (about efficiency) sufficiently well informed? No offence but I assume you to be a mutimedia guru, not a standard library expert, which the author of the code is. We shall not blindly accept authority of course. If you have a suggestion for improvement, given a reasonable ground it would be useful on the musl mailing list. Here and without a real ground it looks like a FUD. As for readability, it is your personal opinion. Again: no offence! Standard libraries are just a quite different area, it postulated other skills and presents other implementation challenges than multimedia programming. Friendly regards, Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
On Mon, Oct 3, 2016 at 7:24 PM, wrote: > On Mon, Oct 03, 2016 at 05:27:19PM +0200, wm4 wrote: >> > Musl merely showed you the problem and now you are suggesting to "document >> > that the messenger with his bad news about our health is no longer >> > welcome". >> >> musl could also choose to abandon its incredibly "clever" hack (that >> makes almost everyone who sees it go "WTF"), and, as was pointed our on > > :-D > IOW it exposes the _bugs_ in how people are accustomed to reason. > >> IRC, probably increase the efficiency and readability of the code in >> question. Yes, musl is technically in the right, but only technically. > > Is your opinion (about efficiency) sufficiently well informed? > No offence but I assume you to be a mutimedia guru, not a standard > library expert, which the author of the code is. > > We shall not blindly accept authority of course. If you have a suggestion > for improvement, given a reasonable ground it would be useful on the musl > mailing list. > > Here and without a real ground it looks like a FUD. > > As for readability, it is your personal opinion. > > Again: no offence! Standard libraries are just a quite different area, > it postulated other skills and presents other implementation challenges > than multimedia programming. > Optimized code is the same everywhere, you just write different algorithms. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]doc/platform: Mention musl where x86_32 is not supported
On Mon, Oct 03, 2016 at 07:28:08PM +0200, Hendrik Leppkes wrote: > > Again: no offence! Standard libraries are just a quite different area, > > it postulated other skills and presents other implementation challenges > > than multimedia programming. > > Optimized code is the same everywhere, you just write different algorithms. Note "the same everywhere". Optimization goals, constraints and strategies are different in different domains. No one is an expert in all of the various domains. Not me, not you. Yes it is hard to accept this, especially for very bright people who really know exceptionally many things - but "a lot" is not enough for everywhere. I hope we can stop discussing musl internals, there is no reason to do it on this list, nor other prerequisits for the discussion to make sense. As a concerned user of ffmpeg on 32-bit Intel I plead to make ffmpeg fully usable on this platform regardless of the internals of the C library being used. IOW I plead to make ffmpeg conform to the standards. My expectation and hope is that this can be done without making a noticeable performance sacrifice. Relying on C library internals is a hack, and not a robust one. Yours, Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] aacenc: WIP support for PCEs
Hopefully whoever wants to have support for crazy formats can help. The table in aacenc.h (temporary position) tells the encoder what to put in the bitstream and how to encode. Problem is, the specifications dont specify anything. Thats because I've not been able to find any bloody specifications and had to work with what the decoder does. And there was plenty of guessing there because the decoder does some magic on layout_map which I can't even figure out nor even know if its correct (it seems to be for the formats I've tested). Then there's the problem with the exact order that the channels have to be in. Again a guessing game, since you essentially have no idea what the index part of the map is supposed to be, whether it has to be incremented starting from the first channel or reset upon every front/side/back channel groups. At least the map to instruct the encoder's straightforward. Anyway, help appreciated. Applies cleanly on 543142990b6f7b8757753c13ea6dbc56275c5c7e, but should work fine with newer versions. --- libavcodec/aacenc.c| 69 +- libavcodec/aacenc.h| 50 +++- libavcodec/aacenctab.h | 13 +- 3 files changed, 124 insertions(+), 8 deletions(-) diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c index 2653cef..c138699 100644 --- a/libavcodec/aacenc.c +++ b/libavcodec/aacenc.c @@ -50,6 +50,40 @@ static AVOnce aac_table_init = AV_ONCE_INIT; +static void put_pce(PutBitContext *pb, AVCodecContext *avctx) +{ +int i, j; +AACEncContext *s = avctx->priv_data; +AACPCEInfo *pce = &s->pce; + +put_bits(pb, 4, 0); + +put_bits(pb, 2, avctx->profile); +put_bits(pb, 4, s->samplerate_index); + +put_bits(pb, 4, pce->num_ele[0]); /* Front */ +put_bits(pb, 4, pce->num_ele[1]); /* Side */ +put_bits(pb, 4, pce->num_ele[2]); /* Back */ +put_bits(pb, 2, pce->num_ele[3]); /* LFE */ +put_bits(pb, 3, 0); /* Assoc data */ +put_bits(pb, 4, 0); /* CCs */ + +put_bits(pb, 1, 0); /* Stereo mixdown */ +put_bits(pb, 1, 0); /* Mono mixdown */ +put_bits(pb, 1, 0); /* Something else */ + +for (i = 0; i < 4; i++) { +for (j = 0; j < pce->num_ele[i]; j++) { +if (i < 3) +put_bits(pb, 1, pce->pairing[i][j]); +put_bits(pb, 4, pce->index[i][j]); +} +} + +avpriv_align_put_bits(pb); +put_bits(pb, 8, 0); +} + /** * Make AAC audio config object. * @see 1.6.2.1 "Syntax - AudioSpecificConfig" @@ -58,7 +92,7 @@ static void put_audio_specific_config(AVCodecContext *avctx) { PutBitContext pb; AACEncContext *s = avctx->priv_data; -int channels = s->channels - (s->channels == 8 ? 1 : 0); +int channels = (!s->needs_pce)*(s->channels - (s->channels == 8 ? 1 : 0)); init_put_bits(&pb, avctx->extradata, avctx->extradata_size); put_bits(&pb, 5, s->profile+1); //profile @@ -68,6 +102,8 @@ static void put_audio_specific_config(AVCodecContext *avctx) put_bits(&pb, 1, 0); //frame length - 1024 samples put_bits(&pb, 1, 0); //does not depend on core coder put_bits(&pb, 1, 0); //is not extension +if (s->needs_pce) +put_pce(&pb, avctx); //Explicitly Mark SBR absent put_bits(&pb, 11, 0x2b7); //sync extension @@ -488,7 +524,7 @@ static void copy_input_samples(AACEncContext *s, const AVFrame *frame) { int ch; int end = 2048 + (frame ? frame->nb_samples : 0); -const uint8_t *channel_map = aac_chan_maps[s->channels - 1]; +const uint8_t *channel_map = s->reorder_map; /* copy and remap input samples */ for (ch = 0; ch < s->channels; ch++) { @@ -923,16 +959,36 @@ static av_cold int aac_encode_init(AVCodecContext *avctx) /* Constants */ s->last_frame_pb_count = 0; -avctx->extradata_size = 5; +avctx->extradata_size = 20; avctx->frame_size = 1024; avctx->initial_padding = 1024; s->lambda = avctx->global_quality > 0 ? avctx->global_quality : 120; /* Channel map and unspecified bitrate guessing */ s->channels = avctx->channels; -ERROR_IF(s->channels > AAC_MAX_CHANNELS || s->channels == 7, - "Unsupported number of channels: %d\n", s->channels); -s->chan_map = aac_chan_configs[s->channels-1]; + +s->needs_pce = 1; +for (i = 0; i < FF_ARRAY_ELEMS(aac_normal_chan_layouts); i++) { +if (avctx->channel_layout == aac_normal_chan_layouts[i]) { +s->needs_pce = s->options.pce; +break; +} +} + +if (s->needs_pce) { +for (i = 0; i < FF_ARRAY_ELEMS(aac_pce_configs); i++) +if (avctx->channel_layout == aac_pce_configs[i].layout) +break; +ERROR_IF(i == FF_ARRAY_ELEMS(aac_pce_configs), "Unsupported channel layout\n"); +WARN_IF(1, "Using a PCE!\n"); +s->pce = aac_pce_configs[i]; +s->reorder_map = s->pce.reorder_map; +s->chan_map = s->pce.config_map; +} else
Re: [FFmpeg-devel] lurking bugs in the mmx-related assembler code (?)
Le duodi 12 vendémiaire, an CCXXV, u-9...@aetey.se a écrit : > The author of the referred code acts in his actual area of competence > (C libraries and standards). > > The comments here on the C library code and standard compliance come from > developers having a different competence area (multimedia programming). > > As bright as the people here are, they land in a foreign area, which > accidentally leads to statements like the above. I am sorry, but an appeal to authority will not cut it in front of real arguments. The real argument here is: musl makes several assumptions about the architecture and compiler (for example: that floats and ints have the same endianness) that are not mandated by any standard. And although these assumptions are very reasonable and widely respected, there will eventually be an architecture or, more probably, a compiler, that will break them. FFmpeg does exactly the same. Now, when these assumptions break, who is to blame? One could say that, by principle, whoever made an assumption not guaranteed by a standard is guilty, but this is a simplistic approach. More reasonably, the one to blame is the one who has the worse reason for making the assumption or breaking it. In this instance, we know the reason for FFmpeg: resetting the FPU state is expensive, and this is speed-critical code. Please tell us what are musl's reasons for using this ugly hack. If the answer is speed, I would very much like to see a benchmark comparing this implementation to a portable but optimized log2, especially for x86_32, where int <-> float conversions are very expensive. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] lurking bugs in the mmx-related assembler code (?)
On Mon, Oct 03, 2016 at 09:48:47PM +0200, Nicolas George wrote: > Le duodi 12 vendémiaire, an CCXXV, u-9...@aetey.se a écrit : > > The author of the referred code acts in his actual area of competence > > (C libraries and standards). > > > > The comments here on the C library code and standard compliance come from > > developers having a different competence area (multimedia programming). > > > > As bright as the people here are, they land in a foreign area, which > > accidentally leads to statements like the above. > > I am sorry, but an appeal to authority will not cut it in front of real > arguments. > > The real argument here is: musl makes several assumptions about the > architecture and compiler (for example: that floats and ints have the same > endianness) that are not mandated by any standard. And although these > assumptions are very reasonable and widely respected, there will eventually > be an architecture or, more probably, a compiler, that will break them. > > FFmpeg does exactly the same. > TBH, having a set of supported architectures in a libc is much more legit than a set of supported libc in a multimedia framework IMO. We can mess as much as we want within our codebase wrt ABI violations, but I don't think it's reasonable to make random assumptions about libc (and others external libraries) implementations. It's way too risky. Even if we are able today to change musl implementation, seeing random floats usage in the many allocators out there doesn't look that surprising. Actually, if I was a valgrind developers, maybe I would do that on purpose... [...] > In this instance, we know the reason for FFmpeg: resetting the FPU state is > expensive, and this is speed-critical code. We have mallocs in inner loops of speed-critical? Really? [...] -- Clément B. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [GSoC] avcodec/als: Add ALS encoder
Hi, > Patch attached. > > It fixes the fate tests. > However, there's a slight bug in the encoder in handling the last frame. > I'll definitely fix it later. > I hope the patch can be merged in this state. no. The last frame has to be handled properly before merging happens. I'm at LinuxCon & ELCE for the next two weeks, so don't expect too much input from me before Oct. 14th. Please fix whatever is possible until then. -Thilo > - Umair > > On Mon, Sep 12, 2016 at 12:53 PM, Umair Khan wrote: > >> Hi, >> >> Sorry for late reply. I was travelling a bit. >> Attached is the patch which includes the following changes than the >> previous one - >> >> 1. Removed changes of the file libavformat/movenc.c, as I had added >> this file in the initial commit by mistake. >> 2. Removed the assembly code. >> 3. Make changes as suggested by Michael (av_assertX and header_size). >> >> As far as fate tests are concerned, I haven't checked that yet. I'll >> see what's the issue there. >> >> On Mon, Aug 29, 2016 at 11:34 PM, Michael Niedermayer >> wrote: >>> On Mon, Aug 29, 2016 at 10:47:59PM +0530, Umair Khan wrote: Hi, On Sun, Aug 28, 2016 at 4:26 PM, Michael Niedermayer wrote: > On Sun, Aug 28, 2016 at 01:34:46PM +0530, Umair Khan wrote: >> Hi, >> >> Patches attached. :) >> >> - Umair > >> Changelog |1 + >> 1 file changed, 1 insertion(+) >> d3f30e62d803d967bd5c27dc5dfad278ce5c02e9 >> 0001-Changelog-Add-entry-for-ALS-encoder.patch >> From 020370545a82c8c1304ec9c177a75e27e59b84e8 Mon Sep 17 00:00:00 >> 2001 >> From: Umair Khan >> Date: Sat, 27 Aug 2016 22:22:02 +0530 >> Subject: [PATCH 1/2] Changelog: Add entry for ALS encoder >> >> Signed-off-by: Umair Khan >> --- >> Changelog | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Changelog b/Changelog >> index b903e31..90c15ad 100644 >> --- a/Changelog >> +++ b/Changelog >> @@ -15,6 +15,7 @@ version : >> - True Audio (TTA) muxer >> - crystalizer audio filter >> - acrusher audio filter >> +- ALS encoder >> >> >> version 3.1: >> -- >> 2.7.4 (Apple Git-66) >> > > [...] >> +static int calc_short_term_prediction(ALSEncContext *ctx, ALSBlock >> *block, >> + int order) >> +{ >> +ALSSpecificConfig *sconf = &ctx->sconf; >> +int i, j; >> + >> +int32_t *res_ptr = block->res_ptr; >> +int32_t *smp_ptr = block->cur_ptr; >> + >> +assert(order > 0); > > should be av_assertX (X=0/1/2) > > > [...] >> +int ff_window_init(WindowContext *wctx, enum WindowType type, int >> length, >> + double param) >> +{ >> +if (!length || length < -1) >> +return AVERROR(EINVAL); >> + >> +wctx->type = type; >> +wctx->length = length; >> +wctx->param = param; >> + >> +switch (type) { >> +case WINDOW_TYPE_RECTANGLE: >> +rectangle_init(wctx); >> +break; >> +case WINDOW_TYPE_WELCH: >> +WINDOW_INIT(welch) >> +break; >> +case WINDOW_TYPE_SINERECT: >> +WINDOW_INIT(sinerect) >> +break; >> +case WINDOW_TYPE_HANNRECT: >> +WINDOW_INIT(hannrect) >> +break; >> +default: >> +return AVERROR(EINVAL); >> +} >> + > >> +if (HAVE_MMX) >> +ff_window_init_mmx(wctx); > > breaks build on non x86 as the function declaration / prototype is > not there in that case What should I do with this then? I'm not too aware of how the whole code works because I didn't originally write it. So, I'll need some help here. :) >>> >>> IIRC the declaration / prototype is under #if >>> but the call is not under #if >>> thus if the condition on the #if is untrue this fails to build >>> >>> (this is not the same as the functions implementation being missing >>> for if(0) code, that one works with all supported platforms) >>> >>> its probably best to split the whole *_mmx code out into a seperate >>> patch and also either the call must be under #if or the declaration >>> must be available independant of an #if >>> >>> >>> [...] >>> -- >>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB >>> >>> Democracy is the form of government in which you can choose your dictator >>> >>> ___ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >> >> >> >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/
Re: [FFmpeg-devel] lurking bugs in the mmx-related assembler code (?)
Hi, On Mon, Oct 3, 2016 at 4:11 PM, Clément Bœsch wrote: > On Mon, Oct 03, 2016 at 09:48:47PM +0200, Nicolas George wrote: > > Le duodi 12 vendémiaire, an CCXXV, u-9...@aetey.se a écrit : > > > The author of the referred code acts in his actual area of competence > > > (C libraries and standards). > > > > > > The comments here on the C library code and standard compliance come > from > > > developers having a different competence area (multimedia programming). > > > > > > As bright as the people here are, they land in a foreign area, which > > > accidentally leads to statements like the above. > > > > I am sorry, but an appeal to authority will not cut it in front of real > > arguments. > > > > The real argument here is: musl makes several assumptions about the > > architecture and compiler (for example: that floats and ints have the > same > > endianness) that are not mandated by any standard. And although these > > assumptions are very reasonable and widely respected, there will > eventually > > be an architecture or, more probably, a compiler, that will break them. > > > > FFmpeg does exactly the same. > > > > TBH, having a set of supported architectures in a libc is much more legit > than a set of supported libc in a multimedia framework IMO. > > We can mess as much as we want within our codebase wrt ABI violations, but > I don't think it's reasonable to make random assumptions about libc (and > others external libraries) implementations. It's way too risky. Even if we > are able today to change musl implementation, seeing random floats usage > in the many allocators out there doesn't look that surprising. > > Actually, if I was a valgrind developers, maybe I would do that on > purpose... > > [...] > > In this instance, we know the reason for FFmpeg: resetting the FPU state > is > > expensive, and this is speed-critical code. > > We have mallocs in inner loops of speed-critical? Really? What code is speed critical? In an audio decoder (like a voice decoder) where a decode call plays as many as a few tens of audio samples at best, I would say that anything is speed critical, because once-per-frame isn't that far off from once-per-sample in that case. The bigger problem is that we're talking about malloc (actually, free, according to cehoyos) here. But this could be any external/libc function. We need emms before any libc call. That's ... Not just mallocs. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] lurking bugs in the mmx-related assembler code (?)
On Mon, Oct 03, 2016 at 04:17:00PM -0400, Ronald S. Bultje wrote: > Hi, > > On Mon, Oct 3, 2016 at 4:11 PM, Clément Bœsch wrote: > > > On Mon, Oct 03, 2016 at 09:48:47PM +0200, Nicolas George wrote: > > > Le duodi 12 vendémiaire, an CCXXV, u-9...@aetey.se a écrit : > > > > The author of the referred code acts in his actual area of competence > > > > (C libraries and standards). > > > > > > > > The comments here on the C library code and standard compliance come > > from > > > > developers having a different competence area (multimedia programming). > > > > > > > > As bright as the people here are, they land in a foreign area, which > > > > accidentally leads to statements like the above. > > > > > > I am sorry, but an appeal to authority will not cut it in front of real > > > arguments. > > > > > > The real argument here is: musl makes several assumptions about the > > > architecture and compiler (for example: that floats and ints have the > > same > > > endianness) that are not mandated by any standard. And although these > > > assumptions are very reasonable and widely respected, there will > > eventually > > > be an architecture or, more probably, a compiler, that will break them. > > > > > > FFmpeg does exactly the same. > > > > > > > TBH, having a set of supported architectures in a libc is much more legit > > than a set of supported libc in a multimedia framework IMO. > > > > We can mess as much as we want within our codebase wrt ABI violations, but > > I don't think it's reasonable to make random assumptions about libc (and > > others external libraries) implementations. It's way too risky. Even if we > > are able today to change musl implementation, seeing random floats usage > > in the many allocators out there doesn't look that surprising. > > > > Actually, if I was a valgrind developers, maybe I would do that on > > purpose... > > > > [...] > > > In this instance, we know the reason for FFmpeg: resetting the FPU state > > is > > > expensive, and this is speed-critical code. > > > > We have mallocs in inner loops of speed-critical? Really? > > > What code is speed critical? In an audio decoder (like a voice decoder) > where a decode call plays as many as a few tens of audio samples at best, I > would say that anything is speed critical, because once-per-frame isn't > that far off from once-per-sample in that case. And then you insert an audio filter using floats and you're doomed? > The bigger problem is that we're talking about malloc (actually, free, > according to cehoyos) here. But this could be any external/libc function. > We need emms before any libc call. That's ... Not just mallocs. Well currently the issue is definitely in the alloc/free, so maybe we should stick to that problem for now? BTW, just another allocator using floats: https://github.com/jemalloc/jemalloc/blob/dev/src/prof.c#L850 -- Clément B. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavc/utils: disallow zero sized packets with data set in avcodec_send_packet
On Fri, 30 Sep 2016, Hendrik Leppkes wrote: On Fri, Sep 30, 2016 at 11:40 AM, wm4 wrote: On Fri, 30 Sep 2016 11:29:05 +0200 Marton Balint wrote: Signed-off-by: Marton Balint --- libavcodec/utils.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavcodec/utils.c b/libavcodec/utils.c index cf85300..d0a6817 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -2813,6 +2813,9 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke if (avctx->internal->draining) return AVERROR_EOF; +if (avpkt && !avpkt->size && avpkt->data) +return AVERROR(EINVAL); + if (!avpkt || !avpkt->size) { avctx->internal->draining = 1; avpkt = NULL; This means packet with size==0 with data!=NULL will be rejected, instead of being interpreted as flush packets. Fine with me. Still allows "normal" flush packets, and probably prevents that API users accidentally enter the EOF state by sending such packets. I agree, such a packet would likely not be meant as a flush packet, and may induce unexpected behavior. Thanks, pushed. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavd/openal: don't return zero sized packet if no samples are available
On Fri, 30 Sep 2016, Marton Balint wrote: Signed-off-by: Marton Balint --- libavdevice/openal-dec.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libavdevice/openal-dec.c b/libavdevice/openal-dec.c index 0647952..6eb0efe 100644 --- a/libavdevice/openal-dec.c +++ b/libavdevice/openal-dec.c @@ -187,9 +187,16 @@ static int read_packet(AVFormatContext* ctx, AVPacket *pkt) const char *error_msg; ALCint nb_samples; -/* Get number of samples available */ -alcGetIntegerv(ad->device, ALC_CAPTURE_SAMPLES, (ALCsizei) sizeof(ALCint), &nb_samples); -if (error = al_get_error(ad->device, &error_msg)) goto fail; +for (;;) { +/* Get number of samples available */ +alcGetIntegerv(ad->device, ALC_CAPTURE_SAMPLES, (ALCsizei) sizeof(ALCint), &nb_samples); +if (error = al_get_error(ad->device, &error_msg)) goto fail; +if (nb_samples > 0) +break; +if (ctx->flags & AVFMT_FLAG_NONBLOCK) +return AVERROR(EAGAIN); +av_usleep(1000); +} /* Create a packet of appropriate size */ if ((error = av_new_packet(pkt, nb_samples*ad->sample_step)) < 0) -- Nobody seemed interested in openal, and the EAGAIN issue pointed out by Nicolas is fixed, so I pushed this. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Developer needed for applying watermark onto video on server
Hi, I am looking for a developer for hire to help us with an issue applying a watermark to videos after they are uploaded and converted on the server. I am the project manager and do not have much technical information but you will be able to speak with our coders directly. Our deadline is roughly 10 days. Please respond with an approximate cost for this project. Thanks, Chuck P. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check
On Sun, Oct 2, 2016 at 6:22 PM, Hendrik Leppkes wrote: > On Sat, Oct 1, 2016 at 4:15 PM, Hendrik Leppkes wrote: >> Decoders have previously not used AVFrame.pts, and with the upcoming >> deprecation of pkt_pts (in favor of pts), this would lead to an errorneous >> interpration of timestamps. >> --- >> ffmpeg.c | 7 +-- >> 1 file changed, 1 insertion(+), 6 deletions(-) >> >> diff --git a/ffmpeg.c b/ffmpeg.c >> index 9a8e65a..cdbf3d4 100644 >> --- a/ffmpeg.c >> +++ b/ffmpeg.c >> @@ -2058,12 +2058,7 @@ static int decode_audio(InputStream *ist, AVPacket >> *pkt, int *got_output) >> } >> } >> >> -/* if the decoder provides a pts, use it instead of the last packet pts. >> - the decoder could be delaying output by a packet or more. */ >> -if (decoded_frame->pts != AV_NOPTS_VALUE) { >> -ist->dts = ist->next_dts = ist->pts = ist->next_pts = >> av_rescale_q(decoded_frame->pts, avctx->time_base, AV_TIME_BASE_Q); >> -decoded_frame_tb = avctx->time_base; >> -} else if (decoded_frame->pkt_pts != AV_NOPTS_VALUE) { >> +if (decoded_frame->pkt_pts != AV_NOPTS_VALUE) { >> decoded_frame->pts = decoded_frame->pkt_pts; >> decoded_frame_tb = ist->st->time_base; >> } else if (pkt->pts != AV_NOPTS_VALUE) { >> -- > > Ping. Last chance for further comments, otherwise I'll apply in the morning (~8 hours from now), so that merges can continue. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/aviobuf.c: Adapt avio_accept and avio_handshake to new AVIOContext API
On Fri, Sep 30, 2016 at 09:17:54PM +0200, Stephan Holljes wrote: > Signed-off-by: Stephan Holljes > --- > libavformat/aviobuf.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > This fixes doc/examples/http_multiclient.c from segfaulting. Seems like these > functions > were forgotten during the white- and blacklisting update. > > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c > index f3acb32..134d627 100644 > --- a/libavformat/aviobuf.c > +++ b/libavformat/aviobuf.c > @@ -1156,7 +1156,8 @@ int avio_read_to_bprint(AVIOContext *h, AVBPrint *pb, > size_t max_size) > int avio_accept(AVIOContext *s, AVIOContext **c) > { > int ret; > -URLContext *sc = s->opaque; > +AVIOInternal *internal = s->opaque; > +URLContext *sc = internal->h; > URLContext *cc = NULL; > ret = ffurl_accept(sc, &cc); > if (ret < 0) > @@ -1166,7 +1167,8 @@ int avio_accept(AVIOContext *s, AVIOContext **c) > > int avio_handshake(AVIOContext *c) > { > -URLContext *cc = c->opaque; > +AVIOInternal *internal = c->opaque; > +URLContext *cc = internal->h; > return ffurl_handshake(cc); > } applied, thanks -- Clément B. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] lurking bugs in the mmx-related assembler code (?)
On Mon, Oct 03, 2016 at 09:48:47PM +0200, Nicolas George wrote: > > As bright as the people here are, they land in a foreign area, which > > accidentally leads to statements like the above. > > I am sorry, but an appeal to authority will not cut it in front of real > arguments. [Everyone, sorry for offtopic] It was not an appeal to authority but a warning, trying to reduce the number of nice and intelligent people making fools of themselves. > The real argument here is: musl makes several assumptions about the > architecture and compiler (for example: that floats and ints have the same You should have noticed that it is actually generated per architecture and has requirements on compilers. > endianness) that are not mandated by any standard. And although these Hmm isn't there such a thing called the CPU specification? > assumptions are very reasonable and widely respected, there will eventually > be an architecture or, more probably, a compiler, that will break them. You mean if you _choose_ to build it for a wrong architecture or by a wrong compiler? Then yes. :) But not otherwise. When built properly it will fully cooperate with applications which conform to standards. Any of them. Unfortunately, this apparently can not be said about ffmpeg. Replacing one standard-conforming library with another one can apparently lead to a crash. > FFmpeg does exactly the same. Alas, not really. > In this instance, we know the reason for FFmpeg: resetting the FPU state is > expensive, and this is speed-critical code. Please tell us what are musl's > reasons for using this ugly hack. If the answer is speed, I would very much Be careful before calling any code "ugly". May be you just do not understand it. And concerning why it looks this way - this is _irrelevant_ here. Don't try to blame the error of the old vp3 code on musl. > like to see a benchmark comparing this implementation to a portable but > optimized log2, especially for x86_32, where int <-> float conversions are > very expensive. You are welcome to learn why the things are done in a certain way. A good place to start might be the musl mailing list. Yours, Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] aacenc: WIP support for PCEs
On Mon, Oct 3, 2016 at 3:53 PM, Rostislav Pehlivanov wrote: > Hopefully whoever wants to have support for crazy formats can help. > The table in aacenc.h (temporary position) tells the encoder what > to put in the bitstream and how to encode. Problem is, the specifications > dont specify anything. Thats because I've not been able to find any bloody > specifications and had to work with what the decoder does. And there was > plenty of guessing there because the decoder does some magic on layout_map > which I can't even figure out nor even know if its correct (it seems to be > for the formats I've tested). > Then there's the problem with the exact order that the channels have to be in. > Again a guessing game, since you essentially have no idea what the index part > of the map is supposed to be, whether it has to be incremented starting from > the first channel or reset upon every front/side/back channel groups. At > least the map to instruct the encoder's straightforward. > > Anyway, help appreciated. Will take a deeper look later, but on a shallow review, shouldn't it have some tests? (seems fairly easily testable) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] lurking bugs in the mmx-related assembler code (?)
On Mon, 3 Oct 2016 23:57:32 +0200 u-9...@aetey.se wrote: please refrain from off topic posts on this mailing list. bug reports go to trac or ffmpeg-user list. see http://ffmpeg.org/bugreports.html "Please do not report your problem on the developer mailing list:" > You are welcome to learn why the things are done in a certain way. > A good place to start might be the musl mailing list. after reading this thread, the thing i realize now is that our developers would rather do a line by line audit of another libc software project than an audit of ffmpeg code :D -compn ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat/hlsenc: support multi level path in m3u8 with filename
Steven Liu 于2016年10月3日 周一上午7:29写道: > will push aftet 24 hours > Steven Liu 于2016年9月26日 周一下午4:05写道: > > > pushed! Thanks ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 0/4] V13 - SCTE-35
- Same as V12 except for an empty that shouldn't have been there. Carlos Fernandez (4): Adding SCTE-35 CUI codec SCTE-35 extraction from mpegts SCTE-35 support in hlsenc Correct Indentation libavcodec/avcodec.h| 2 + libavcodec/codec_desc.c | 6 + libavformat/Makefile| 2 +- libavformat/hlsenc.c| 116 --- libavformat/mpegts.c| 47 - libavformat/scte_35.c | 527 libavformat/scte_35.h | 86 7 files changed, 758 insertions(+), 28 deletions(-) create mode 100644 libavformat/scte_35.c create mode 100644 libavformat/scte_35.h -- 2.7.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/4] V13 - SCTE-35 extraction from mpegts
From: Carlos Fernandez Signed-off-by: Carlos Fernandez --- libavformat/mpegts.c | 47 +-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c index b31d233..932ffc1 100644 --- a/libavformat/mpegts.c +++ b/libavformat/mpegts.c @@ -725,6 +725,12 @@ static const StreamType HDMV_types[] = { { 0 }, }; +/* SCTE types */ +static const StreamType SCTE_types[] = { +{ 0x86, AVMEDIA_TYPE_DATA, AV_CODEC_ID_SCTE_35}, +{ 0 }, +}; + /* ATSC ? */ static const StreamType MISC_types[] = { { 0x81, AVMEDIA_TYPE_AUDIO, AV_CODEC_ID_AC3 }, @@ -872,6 +878,13 @@ static void reset_pes_packet_state(PESContext *pes) av_buffer_unref(&pes->buffer); } +static void new_data_packet(const uint8_t *buffer, int len, AVPacket *pkt) +{ +av_init_packet(pkt); +pkt->data = buffer; +pkt->size = len; +} + static int new_pes_packet(PESContext *pes, AVPacket *pkt) { char *sd; @@ -1590,6 +1603,27 @@ static void m4sl_cb(MpegTSFilter *filter, const uint8_t *section, av_free(mp4_descr[i].dec_config_descr); } +static void scte_data_cb(MpegTSFilter *filter, const uint8_t *section, +int section_len) +{ +AVProgram *prg = NULL; +MpegTSContext *ts = filter->u.section_filter.opaque; + +int idx = ff_find_stream_index(ts->stream, filter->pid); +new_data_packet(section, section_len, ts->pkt); +if (idx >= 0) { +ts->pkt->stream_index = idx; +} +prg = av_find_program_from_stream(ts->stream, NULL, idx); +if (prg && prg->pcr_pid != -1 && prg->discard != AVDISCARD_ALL) { +MpegTSFilter *f = ts->pids[prg->pcr_pid]; +if (f) +ts->pkt->pts = ts->pkt->dts = f->last_pcr/300; +} +ts->stop_parse = 1; + +} + static const uint8_t opus_coupled_stream_cnt[9] = { 1, 0, 1, 1, 2, 2, 2, 3, 3 }; @@ -1868,6 +1902,12 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type return 0; } +static int is_pes_stream(int stream_type, uint32_t prog_reg_desc) +{ +return !(stream_type == 0x13 || + (stream_type == 0x86 && prog_reg_desc == AV_RL32("CUEI")) ); +} + static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len) { MpegTSContext *ts = filter->u.section_filter.opaque; @@ -1975,7 +2015,7 @@ static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len pes->st->id = pes->pid; } st = pes->st; -} else if (stream_type != 0x13) { +} else if (is_pes_stream(stream_type, prog_reg_desc)) { if (ts->pids[pid]) mpegts_close_filter(ts, ts->pids[pid]); // wrongly added sdt filter probably pes = add_pes_stream(ts, pid, pcr_pid); @@ -1995,6 +2035,10 @@ static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len goto out; st->id = pid; st->codecpar->codec_type = AVMEDIA_TYPE_DATA; +if (stream_type == 0x86 && prog_reg_desc == AV_RL32("CUEI")) { +mpegts_find_stream_type(st, stream_type, SCTE_types); +mpegts_open_section_filter(ts, pid, scte_data_cb, ts, 1); +} } } @@ -2317,7 +2361,6 @@ static int handle_packet(MpegTSContext *ts, const uint8_t *packet) } } } - } else { int ret; // Note: The position here points actually behind the current packet. -- 2.7.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/4] V13 - Correct Indentation
From: Carlos Fernandez Signed-off-by: Carlos Fernandez --- libavformat/hlsenc.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index 44259ec..83d6881 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -406,12 +406,12 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double else en->sub_filename[0] = '\0'; -en->duration = duration; -en->pos = pos; -en->event= event; -en->size = size; +en->duration = duration; +en->pos= pos; +en->event = event; +en->size = size; en->start_pts = start_pts; -en->next = NULL; +en->next = NULL; if (hls->scte_iface) { if (hls->scte_iface->event_state == EVENT_OUT_CONT) { -- 2.7.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/4] V13 - Adding SCTE-35 CUI codec
From: Carlos Fernandez Signed-off-by: Carlos Fernandez --- libavcodec/avcodec.h| 2 ++ libavcodec/codec_desc.c | 6 ++ 2 files changed, 8 insertions(+) diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index d72ee07..9064006 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -631,6 +631,8 @@ enum AVCodecID { AV_CODEC_ID_FIRST_UNKNOWN = 0x18000, ///< A dummy ID pointing at the start of various fake codecs. AV_CODEC_ID_TTF = 0x18000, +AV_CODEC_ID_SCTE_35,/**< Contain no valid time stamp in DTS PTS of avpacket, avpacket data contain time stamp + in scte-35 format which is relative to DTS/PTS of video stream */ AV_CODEC_ID_BINTEXT= 0x18800, AV_CODEC_ID_XBIN, AV_CODEC_ID_IDF, diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c index 24948ca..2612215 100644 --- a/libavcodec/codec_desc.c +++ b/libavcodec/codec_desc.c @@ -2964,6 +2964,12 @@ static const AVCodecDescriptor codec_descriptors[] = { .long_name = NULL_IF_CONFIG_SMALL("binary data"), .mime_types= MT("application/octet-stream"), }, +{ +.id= AV_CODEC_ID_SCTE_35, +.type = AVMEDIA_TYPE_DATA, +.name = "scte_35", +.long_name = NULL_IF_CONFIG_SMALL("SCTE 35 Message Queue"), +}, /* deprecated codec ids */ }; -- 2.7.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/4] V13 - SCTE-35 support in hlsenc
From: Carlos Fernandez Signed-off-by: Carlos Fernandez --- libavformat/Makefile | 2 +- libavformat/hlsenc.c | 108 +-- libavformat/scte_35.c | 527 ++ libavformat/scte_35.h | 86 4 files changed, 701 insertions(+), 22 deletions(-) create mode 100644 libavformat/scte_35.c create mode 100644 libavformat/scte_35.h diff --git a/libavformat/Makefile b/libavformat/Makefile index 5d827d31..9218606 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -205,7 +205,7 @@ OBJS-$(CONFIG_HDS_MUXER) += hdsenc.o OBJS-$(CONFIG_HEVC_DEMUXER) += hevcdec.o rawdec.o OBJS-$(CONFIG_HEVC_MUXER)+= rawenc.o OBJS-$(CONFIG_HLS_DEMUXER) += hls.o -OBJS-$(CONFIG_HLS_MUXER) += hlsenc.o +OBJS-$(CONFIG_HLS_MUXER) += hlsenc.o scte_35.o OBJS-$(CONFIG_HNM_DEMUXER) += hnm.o OBJS-$(CONFIG_ICO_DEMUXER) += icodec.o OBJS-$(CONFIG_ICO_MUXER) += icoenc.o diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index c161937..44259ec 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -38,6 +38,7 @@ #include "avio_internal.h" #include "internal.h" #include "os_support.h" +#include "scte_35.h" #define KEYSIZE 16 #define LINE_BUFFER_SIZE 1024 @@ -48,6 +49,10 @@ typedef struct HLSSegment { double duration; /* in seconds */ int64_t pos; int64_t size; +struct scte_35_event *event; +int out; +int adv_count; +int64_t start_pts; char key_uri[LINE_BUFFER_SIZE + 1]; char iv_string[KEYSIZE*2 + 1]; @@ -92,6 +97,8 @@ typedef struct HLSContext { uint32_t flags;// enum HLSFlags uint32_t pl_type; // enum PlaylistType char *segment_filename; +char *adv_filename; +char *adv_subfilename; int use_localtime; ///< flag to expand filename with localtime int use_localtime_mkdir;///< flag to mkdir dirname in timebased filename @@ -108,6 +115,8 @@ typedef struct HLSContext { int nb_entries; int discontinuity_set; +int adv_count; +struct scte_35_interface *scte_iface; HLSSegment *segments; HLSSegment *last_segment; HLSSegment *old_segments; @@ -241,6 +250,8 @@ static int hls_delete_old_segments(HLSContext *hls) { av_freep(&path); previous_segment = segment; segment = previous_segment->next; +if (hls->scte_iface) +hls->scte_iface->unref_scte35_event(&previous_segment->event); av_free(previous_segment); } @@ -360,8 +371,8 @@ static int hls_mux_init(AVFormatContext *s) } /* Create a new segment and append it to the segment list */ -static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double duration, - int64_t pos, int64_t size) +static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double duration, int64_t pos, + int64_t start_pts, struct scte_35_event *event, int64_t size) { HLSSegment *en = av_malloc(sizeof(*en)); char *tmp, *p; @@ -397,9 +408,23 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double en->duration = duration; en->pos = pos; +en->event= event; en->size = size; +en->start_pts = start_pts; en->next = NULL; +if (hls->scte_iface) { +if (hls->scte_iface->event_state == EVENT_OUT_CONT) { +en->adv_count = hls->adv_count;; +hls->adv_count++; +en->out = hls->scte_iface->event_state; +} else { +hls->adv_count = 0; +en->out = hls->scte_iface->event_state; +} +} + + if (hls->key_info_file) { av_strlcpy(en->key_uri, hls->key_uri, sizeof(en->key_uri)); av_strlcpy(en->iv_string, hls->iv_string, sizeof(en->iv_string)); @@ -473,7 +498,7 @@ static int parse_playlist(AVFormatContext *s, const char *url) new_start_pos = avio_tell(hls->avf->pb); hls->size = new_start_pos - hls->start_pos; av_strlcpy(hls->avf->filename, line, sizeof(line)); -ret = hls_append_segment(s, hls, hls->duration, hls->start_pos, hls->size); +ret = hls_append_segment(s, hls, hls->duration, hls->start_pos, 0, NULL, hls->size); if (ret < 0) goto fail; hls->start_pos = new_start_pos; @@ -603,9 +628,23 @@ static int hls_window(AVFormatContext *s, int last) avio_printf(out, "#EXT-X-PROGRAM-DATE-TIME:%s.%03d%s\n", buf0, milli, buf1); prog_date_time += en->duration; } -if (hls->baseurl) -avio_printf(out, "%s", hls->baseurl); -avio_printf(out, "%s\n", en->filename); +if (hls->scte_iface && (en->event || en->out) ) { +char *str; +char fname[1024]
[FFmpeg-devel] [PATCH 02/11] avformat/matroskaenc: print debug message with cluster offsets only if the output is seekable
Printing the dynamic buffer offset is useless. Signed-off-by: James Almer --- libavformat/matroskaenc.c | 22 +++--- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 32d5dcf..501bab2 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1934,10 +1934,15 @@ static void mkv_start_new_cluster(AVFormatContext *s, AVPacket *pkt) pb = mkv->dyn_bc; } -av_log(s, AV_LOG_DEBUG, -"Starting new cluster at offset %" PRIu64 " bytes, " -"pts %" PRIu64 "dts %" PRIu64 "\n", -avio_tell(pb), pkt->pts, pkt->dts); +if (s->pb->seekable) +av_log(s, AV_LOG_DEBUG, + "Starting new cluster at offset %" PRIu64 " bytes, " + "pts %" PRIu64 "dts %" PRIu64 "\n", + avio_tell(s->pb), pkt->pts, pkt->dts); +else +av_log(s, AV_LOG_DEBUG, "Starting new cluster, " + "pts %" PRIu64 "dts %" PRIu64 "\n", + pkt->pts, pkt->dts); end_ebml_master(pb, mkv->cluster); mkv->cluster_pos = -1; if (mkv->dyn_bc) @@ -2120,9 +2125,12 @@ static int mkv_write_flush_packet(AVFormatContext *s, AVPacket *pkt) pb = mkv->dyn_bc; if (!pkt) { if (mkv->cluster_pos != -1) { -av_log(s, AV_LOG_DEBUG, - "Flushing cluster at offset %" PRIu64 " bytes\n", - avio_tell(pb)); +if (s->pb->seekable) +av_log(s, AV_LOG_DEBUG, + "Flushing cluster at offset %" PRIu64 " bytes\n", + avio_tell(s->pb)); +else +av_log(s, AV_LOG_DEBUG, "Flushing cluster\n"); end_ebml_master(pb, mkv->cluster); mkv->cluster_pos = -1; if (mkv->dyn_bc) -- 2.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 00/11] CRC32 support for Matroska muxer
This patchsets implements the feature requested on ticket #4347. The first three patches are preparation work. The first one isn't strictly related to the implementation, but comes in handy nonetheless. Patches 4 to 11 can be squashed into a single commit before pushing if that's prefered, but for review purposes i split things as one patch per Level 1 element being adapted to write a CRC32 element. Fate tests are updated as needed. James Almer (11): avformat/matroskaenc: don't reserve space for stream duration tags if the output is not seekable avformat/matroskaenc: print debug message with cluster offsets only if the output is seekable avformat/matroskaenc: always use a dynamic buffer when writting clusters avformat/matroskaenc: write a CRC32 element on each Cluster avformat/matroskaenc: write a CRC32 element on SeekHead avformat/matroskaenc: write a CRC32 element on Cues avformat/matroskaenc: write a CRC32 element on Tracks avformat/matroskaenc: write a CRC32 element on Chapters avformat/matroskaenc: write a CRC32 element on Attachments avformat/matroskaenc: write a CRC32 element on Tags avformat/matroskaenc: write a CRC32 element on Info libavformat/matroskaenc.c| 338 --- tests/fate/matroska.mak | 2 +- tests/fate/wavpack.mak | 4 +- tests/ref/fate/binsub-mksenc | 2 +- tests/ref/fate/rgb24-mkv | 4 +- tests/ref/lavf/mka | 4 +- tests/ref/lavf/mkv | 8 +- tests/ref/seek/lavf-mkv | 44 +++--- 8 files changed, 225 insertions(+), 181 deletions(-) -- 2.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 01/11] avformat/matroskaenc: don't reserve space for stream duration tags if the output is not seekable
The durations are never written in that situation. Signed-off-by: James Almer --- libavformat/matroskaenc.c| 2 +- tests/fate/matroska.mak | 2 +- tests/fate/wavpack.mak | 4 ++-- tests/ref/fate/binsub-mksenc | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 3eeb09b..32d5dcf 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1376,7 +1376,7 @@ static int mkv_write_tags(AVFormatContext *s) if (ret < 0) return ret; } -if (!mkv->is_live) { +if (s->pb->seekable && !mkv->is_live) { for (i = 0; i < s->nb_streams; i++) { ebml_master tag_target; ebml_master tag; diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak index 8e4a1e8..36cc779 100644 --- a/tests/fate/matroska.mak +++ b/tests/fate/matroska.mak @@ -4,6 +4,6 @@ FATE_MATROSKA-$(call DEMMUX, MATROSKA, MATROSKA) += fate-matroska-remux fate-matroska-remux: CMD = md5 -i $(TARGET_SAMPLES)/vp9-test-vectors/vp90-2-2pass-akiyo.webm -color_trc 4 -c:v copy -fflags +bitexact -strict -2 -f matroska fate-matroska-remux: CMP = oneline -fate-matroska-remux: REF = f08b20b90f158a4de5a02a52c25596b9 +fate-matroska-remux: REF = 1040692ffdfee2428954af79a7d5d155 FATE_SAMPLES_AVCONV += $(FATE_MATROSKA-yes) diff --git a/tests/fate/wavpack.mak b/tests/fate/wavpack.mak index a825a02..240f5ea 100644 --- a/tests/fate/wavpack.mak +++ b/tests/fate/wavpack.mak @@ -91,12 +91,12 @@ fate-wavpack-matroskamode: CMD = md5 -i $(TARGET_SAMPLES)/wavpack/special/matros FATE_WAVPACK-$(call DEMMUX, WV, MATROSKA) += fate-wavpack-matroska_mux-mono fate-wavpack-matroska_mux-mono: CMD = md5 -i $(TARGET_SAMPLES)/wavpack/num_channels/mono_16bit_int.wv -c copy -fflags +bitexact -f matroska fate-wavpack-matroska_mux-mono: CMP = oneline -fate-wavpack-matroska_mux-mono: REF = 4befcc41dab6c690a15d0c396c324468 +fate-wavpack-matroska_mux-mono: REF = a2987e2e51e01a35e47e7da13eb47a35 FATE_WAVPACK-$(call DEMMUX, WV, MATROSKA) += fate-wavpack-matroska_mux-61 fate-wavpack-matroska_mux-61: CMD = md5 -i $(TARGET_SAMPLES)/wavpack/num_channels/eva_2.22_6.1_16bit-partial.wv -c copy -fflags +bitexact -f matroska fate-wavpack-matroska_mux-61: CMP = oneline -fate-wavpack-matroska_mux-61: REF = 7fedbfc3b9ea7348761db664626c29f4 +fate-wavpack-matroska_mux-61: REF = ffba4ddea1ba71f7a5901d9ed1a267be FATE_SAMPLES_AVCONV += $(FATE_WAVPACK-yes) fate-wavpack: $(FATE_WAVPACK-yes) diff --git a/tests/ref/fate/binsub-mksenc b/tests/ref/fate/binsub-mksenc index 128ca31..c473497 100644 --- a/tests/ref/fate/binsub-mksenc +++ b/tests/ref/fate/binsub-mksenc @@ -1 +1 @@ -37a212f8d56ad71e7466d5129f88e756 +2dad5f63688ec613a04e94c8d4d167db -- 2.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 03/11] avformat/matroskaenc: always use a dynamic buffer when writting clusters
Signed-off-by: James Almer --- libavformat/matroskaenc.c | 55 ++- 1 file changed, 16 insertions(+), 39 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 501bab2..2f56d61 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1914,9 +1914,6 @@ static void mkv_flush_dynbuf(AVFormatContext *s) int bufsize; uint8_t *dyn_buf; -if (!mkv->dyn_bc) -return; - bufsize = avio_close_dyn_buf(mkv->dyn_bc, &dyn_buf); avio_write(s->pb, dyn_buf, bufsize); av_free(dyn_buf); @@ -1926,14 +1923,11 @@ static void mkv_flush_dynbuf(AVFormatContext *s) static void mkv_start_new_cluster(AVFormatContext *s, AVPacket *pkt) { MatroskaMuxContext *mkv = s->priv_data; -AVIOContext *pb; - -if (s->pb->seekable) { -pb = s->pb; -} else { -pb = mkv->dyn_bc; -} +AVIOContext *pb = mkv->dyn_bc; +end_ebml_master(pb, mkv->cluster); +mkv->cluster_pos = -1; +mkv_flush_dynbuf(s); if (s->pb->seekable) av_log(s, AV_LOG_DEBUG, "Starting new cluster at offset %" PRIu64 " bytes, " @@ -1943,10 +1937,6 @@ static void mkv_start_new_cluster(AVFormatContext *s, AVPacket *pkt) av_log(s, AV_LOG_DEBUG, "Starting new cluster, " "pts %" PRIu64 "dts %" PRIu64 "\n", pkt->pts, pkt->dts); -end_ebml_master(pb, mkv->cluster); -mkv->cluster_pos = -1; -if (mkv->dyn_bc) -mkv_flush_dynbuf(s); avio_flush(s->pb); } @@ -1976,16 +1966,14 @@ static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt, int add_ } } -if (!s->pb->seekable) { -if (!mkv->dyn_bc) { -ret = avio_open_dyn_buf(&mkv->dyn_bc); -if (ret < 0) { -av_log(s, AV_LOG_ERROR, "Failed to open dynamic buffer\n"); -return ret; -} +if (!mkv->dyn_bc) { +ret = avio_open_dyn_buf(&mkv->dyn_bc); +if (ret < 0) { +av_log(s, AV_LOG_ERROR, "Failed to open dynamic buffer\n"); +return ret; } -pb = mkv->dyn_bc; } +pb = mkv->dyn_bc; if (mkv->cluster_pos == -1) { mkv->cluster_pos = avio_tell(s->pb); @@ -1994,7 +1982,7 @@ static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt, int add_ mkv->cluster_pts = FFMAX(0, ts); } -relative_packet_pos = avio_tell(s->pb) - mkv->cluster.pos; +relative_packet_pos = avio_tell(pb) - mkv->cluster.pos; if (par->codec_type != AVMEDIA_TYPE_SUBTITLE) { mkv_write_block(s, pb, MATROSKA_ID_SIMPLEBLOCK, pkt, keyframe); @@ -2058,11 +2046,7 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt) // start a new cluster every 5 MB or 5 sec, or 32k / 1 sec for streaming or // after 4k and on a keyframe -if (s->pb->seekable) { -cluster_size = avio_tell(s->pb) - mkv->cluster_pos; -} else { -cluster_size = avio_tell(mkv->dyn_bc); -} +cluster_size = avio_tell(mkv->dyn_bc); if (mkv->is_dash && codec_type == AVMEDIA_TYPE_VIDEO) { // WebM DASH specification states that the first block of every cluster @@ -2118,23 +2102,18 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt) static int mkv_write_flush_packet(AVFormatContext *s, AVPacket *pkt) { MatroskaMuxContext *mkv = s->priv_data; -AVIOContext *pb; -if (s->pb->seekable) -pb = s->pb; -else -pb = mkv->dyn_bc; +AVIOContext *pb = mkv->dyn_bc; if (!pkt) { if (mkv->cluster_pos != -1) { +end_ebml_master(pb, mkv->cluster); +mkv->cluster_pos = -1; +mkv_flush_dynbuf(s); if (s->pb->seekable) av_log(s, AV_LOG_DEBUG, "Flushing cluster at offset %" PRIu64 " bytes\n", avio_tell(s->pb)); else av_log(s, AV_LOG_DEBUG, "Flushing cluster\n"); -end_ebml_master(pb, mkv->cluster); -mkv->cluster_pos = -1; -if (mkv->dyn_bc) -mkv_flush_dynbuf(s); avio_flush(s->pb); } return 1; @@ -2163,8 +2142,6 @@ static int mkv_write_trailer(AVFormatContext *s) if (mkv->dyn_bc) { end_ebml_master(mkv->dyn_bc, mkv->cluster); mkv_flush_dynbuf(s); -} else if (mkv->cluster_pos != -1) { -end_ebml_master(pb, mkv->cluster); } if (mkv->mode != MODE_WEBM) { -- 2.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 04/11] avformat/matroskaenc: write a CRC32 element on each Cluster
Implements part of ticket #4347 Signed-off-by: James Almer --- libavformat/matroskaenc.c | 80 +-- tests/ref/fate/rgb24-mkv | 4 +-- tests/ref/lavf/mka| 4 +-- tests/ref/lavf/mkv| 8 ++--- tests/ref/seek/lavf-mkv | 44 +- 5 files changed, 79 insertions(+), 61 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 2f56d61..1bbf6c6 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -37,6 +37,7 @@ #include "libavutil/avstring.h" #include "libavutil/channel_layout.h" +#include "libavutil/crc.h" #include "libavutil/dict.h" #include "libavutil/intfloat.h" #include "libavutil/intreadwrite.h" @@ -305,6 +306,45 @@ static void end_ebml_master(AVIOContext *pb, ebml_master master) avio_seek(pb, pos, SEEK_SET); } +static int start_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, ebml_master *master, + unsigned int elementid, uint64_t expectedsize) +{ +int ret; + +if (ret = avio_open_dyn_buf(dyn_cp) < 0) +return ret; + +if (pb->seekable) +*master = start_ebml_master(pb, elementid, expectedsize); +else +*master = start_ebml_master(*dyn_cp, elementid, expectedsize); + +return 0; +} + +static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, MatroskaMuxContext *mkv, + ebml_master master) +{ +uint8_t *buf, crc[4]; +int size; + +if (pb->seekable) { +size = avio_close_dyn_buf(*dyn_cp, &buf); +if (mkv->mode != MODE_WEBM) { +AV_WL32(crc, av_crc(av_crc_get_table(AV_CRC_32_IEEE_LE), UINT32_MAX, buf, size) ^ UINT32_MAX); +put_ebml_binary(pb, EBML_ID_CRC32, crc, sizeof(crc)); +} +avio_write(pb, buf, size); +end_ebml_master(pb, master); +} else { +end_ebml_master(*dyn_cp, master); +size = avio_close_dyn_buf(*dyn_cp, &buf); +avio_write(pb, buf, size); +} +av_free(buf); +*dyn_cp = NULL; +} + static void put_xiph_size(AVIOContext *pb, int size) { ffio_fill(pb, 255, size / 255); @@ -1908,26 +1948,12 @@ static int mkv_write_vtt_blocks(AVFormatContext *s, AVIOContext *pb, AVPacket *p return pkt->duration; } -static void mkv_flush_dynbuf(AVFormatContext *s) -{ -MatroskaMuxContext *mkv = s->priv_data; -int bufsize; -uint8_t *dyn_buf; - -bufsize = avio_close_dyn_buf(mkv->dyn_bc, &dyn_buf); -avio_write(s->pb, dyn_buf, bufsize); -av_free(dyn_buf); -mkv->dyn_bc = NULL; -} - static void mkv_start_new_cluster(AVFormatContext *s, AVPacket *pkt) { MatroskaMuxContext *mkv = s->priv_data; -AVIOContext *pb = mkv->dyn_bc; -end_ebml_master(pb, mkv->cluster); +end_ebml_master_crc32(s->pb, &mkv->dyn_bc, mkv, mkv->cluster); mkv->cluster_pos = -1; -mkv_flush_dynbuf(s); if (s->pb->seekable) av_log(s, AV_LOG_DEBUG, "Starting new cluster at offset %" PRIu64 " bytes, " @@ -1966,21 +1992,15 @@ static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt, int add_ } } -if (!mkv->dyn_bc) { -ret = avio_open_dyn_buf(&mkv->dyn_bc); -if (ret < 0) { -av_log(s, AV_LOG_ERROR, "Failed to open dynamic buffer\n"); -return ret; -} -} -pb = mkv->dyn_bc; - if (mkv->cluster_pos == -1) { mkv->cluster_pos = avio_tell(s->pb); -mkv->cluster = start_ebml_master(pb, MATROSKA_ID_CLUSTER, 0); -put_ebml_uint(pb, MATROSKA_ID_CLUSTERTIMECODE, FFMAX(0, ts)); +ret = start_ebml_master_crc32(s->pb, &mkv->dyn_bc, &mkv->cluster, MATROSKA_ID_CLUSTER, 0); +if (ret < 0) +return ret; +put_ebml_uint(mkv->dyn_bc, MATROSKA_ID_CLUSTERTIMECODE, FFMAX(0, ts)); mkv->cluster_pts = FFMAX(0, ts); } +pb = mkv->dyn_bc; relative_packet_pos = avio_tell(pb) - mkv->cluster.pos; @@ -2102,12 +2122,11 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt) static int mkv_write_flush_packet(AVFormatContext *s, AVPacket *pkt) { MatroskaMuxContext *mkv = s->priv_data; -AVIOContext *pb = mkv->dyn_bc; + if (!pkt) { if (mkv->cluster_pos != -1) { -end_ebml_master(pb, mkv->cluster); +end_ebml_master_crc32(s->pb, &mkv->dyn_bc, mkv, mkv->cluster); mkv->cluster_pos = -1; -mkv_flush_dynbuf(s); if (s->pb->seekable) av_log(s, AV_LOG_DEBUG, "Flushing cluster at offset %" PRIu64 " bytes\n", @@ -2140,8 +2159,7 @@ static int mkv_write_trailer(AVFormatContext *s) } if (mkv->dyn_bc) { -end_ebml_master(mkv->dyn_bc, mkv->cluster); -mkv_flush_dynbuf(s); +end_ebml_master_crc32(pb, &mkv->dyn_bc, mkv, mkv->cluster); } if (mkv->mode != MODE_WEBM) { diff --git a/test
[FFmpeg-devel] [PATCH 05/11] avformat/matroskaenc: write a CRC32 element on SeekHead
Implements part of ticket #4347 Signed-off-by: James Almer --- libavformat/matroskaenc.c| 28 +--- tests/fate/matroska.mak | 2 +- tests/fate/wavpack.mak | 4 ++-- tests/ref/fate/binsub-mksenc | 2 +- tests/ref/fate/rgb24-mkv | 4 ++-- tests/ref/lavf/mka | 4 ++-- tests/ref/lavf/mkv | 8 tests/ref/seek/lavf-mkv | 44 ++-- 8 files changed, 51 insertions(+), 45 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 1bbf6c6..da45f7c 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -391,9 +391,9 @@ static mkv_seekhead *mkv_start_seekhead(AVIOContext *pb, int64_t segment_offset, if (numelements > 0) { new_seekhead->filepos = avio_tell(pb); // 21 bytes max for a seek entry, 10 bytes max for the SeekHead ID -// and size, and 3 bytes to guarantee that an EBML void element -// will fit afterwards -new_seekhead->reserved_size = numelements * MAX_SEEKENTRY_SIZE + 13; +// and size, 6 bytes for a CRC32 element, and 3 bytes to guarantee +// that an EBML void element will fit afterwards +new_seekhead->reserved_size = numelements * MAX_SEEKENTRY_SIZE + 19; new_seekhead->max_entries = numelements; put_ebml_void(pb, new_seekhead->reserved_size); } @@ -430,6 +430,7 @@ static int mkv_add_seekhead_entry(mkv_seekhead *seekhead, unsigned int elementid */ static int64_t mkv_write_seekhead(AVIOContext *pb, MatroskaMuxContext *mkv) { +AVIOContext *dyn_cp; mkv_seekhead *seekhead = mkv->main_seekhead; ebml_master metaseek, seekentry; int64_t currentpos; @@ -444,20 +445,25 @@ static int64_t mkv_write_seekhead(AVIOContext *pb, MatroskaMuxContext *mkv) } } -metaseek = start_ebml_master(pb, MATROSKA_ID_SEEKHEAD, seekhead->reserved_size); +if (start_ebml_master_crc32(pb, &dyn_cp, &metaseek, MATROSKA_ID_SEEKHEAD, +seekhead->reserved_size) < 0) { +currentpos = -1; +goto fail; +} + for (i = 0; i < seekhead->num_entries; i++) { mkv_seekhead_entry *entry = &seekhead->entries[i]; -seekentry = start_ebml_master(pb, MATROSKA_ID_SEEKENTRY, MAX_SEEKENTRY_SIZE); +seekentry = start_ebml_master(dyn_cp, MATROSKA_ID_SEEKENTRY, MAX_SEEKENTRY_SIZE); -put_ebml_id(pb, MATROSKA_ID_SEEKID); -put_ebml_num(pb, ebml_id_size(entry->elementid), 0); -put_ebml_id(pb, entry->elementid); +put_ebml_id(dyn_cp, MATROSKA_ID_SEEKID); +put_ebml_num(dyn_cp, ebml_id_size(entry->elementid), 0); +put_ebml_id(dyn_cp, entry->elementid); -put_ebml_uint(pb, MATROSKA_ID_SEEKPOSITION, entry->segmentpos); -end_ebml_master(pb, seekentry); +put_ebml_uint(dyn_cp, MATROSKA_ID_SEEKPOSITION, entry->segmentpos); +end_ebml_master(dyn_cp, seekentry); } -end_ebml_master(pb, metaseek); +end_ebml_master_crc32(pb, &dyn_cp, mkv, metaseek); if (seekhead->reserved_size > 0) { uint64_t remaining = seekhead->filepos + seekhead->reserved_size - avio_tell(pb); diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak index 36cc779..7de9a59 100644 --- a/tests/fate/matroska.mak +++ b/tests/fate/matroska.mak @@ -4,6 +4,6 @@ FATE_MATROSKA-$(call DEMMUX, MATROSKA, MATROSKA) += fate-matroska-remux fate-matroska-remux: CMD = md5 -i $(TARGET_SAMPLES)/vp9-test-vectors/vp90-2-2pass-akiyo.webm -color_trc 4 -c:v copy -fflags +bitexact -strict -2 -f matroska fate-matroska-remux: CMP = oneline -fate-matroska-remux: REF = 1040692ffdfee2428954af79a7d5d155 +fate-matroska-remux: REF = d1a5fc15908ba10ca3efa282059ca79f FATE_SAMPLES_AVCONV += $(FATE_MATROSKA-yes) diff --git a/tests/fate/wavpack.mak b/tests/fate/wavpack.mak index 240f5ea..32ae3f6 100644 --- a/tests/fate/wavpack.mak +++ b/tests/fate/wavpack.mak @@ -91,12 +91,12 @@ fate-wavpack-matroskamode: CMD = md5 -i $(TARGET_SAMPLES)/wavpack/special/matros FATE_WAVPACK-$(call DEMMUX, WV, MATROSKA) += fate-wavpack-matroska_mux-mono fate-wavpack-matroska_mux-mono: CMD = md5 -i $(TARGET_SAMPLES)/wavpack/num_channels/mono_16bit_int.wv -c copy -fflags +bitexact -f matroska fate-wavpack-matroska_mux-mono: CMP = oneline -fate-wavpack-matroska_mux-mono: REF = a2987e2e51e01a35e47e7da13eb47a35 +fate-wavpack-matroska_mux-mono: REF = 11773e2a518edc788475f3880d849230 FATE_WAVPACK-$(call DEMMUX, WV, MATROSKA) += fate-wavpack-matroska_mux-61 fate-wavpack-matroska_mux-61: CMD = md5 -i $(TARGET_SAMPLES)/wavpack/num_channels/eva_2.22_6.1_16bit-partial.wv -c copy -fflags +bitexact -f matroska fate-wavpack-matroska_mux-61: CMP = oneline -fate-wavpack-matroska_mux-61: REF = ffba4ddea1ba71f7a5901d9ed1a267be +fate-wavpack-matroska_mux-61: REF = 9641abdf596c10c2e21bd9b026d4bade FATE_SAMPLES_AVCONV += $(FATE_WAVPACK-yes) fate-wavpack: $(FATE_WAVPAC
[FFmpeg-devel] [PATCH 09/11] avformat/matroskaenc: write a CRC32 element on Attachments
Implements part of ticket #4347 Signed-off-by: James Almer --- libavformat/matroskaenc.c | 21 +++-- tests/ref/lavf/mkv| 4 ++-- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 9687833..c63ecdd 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1467,7 +1467,7 @@ static int mkv_write_tags(AVFormatContext *s) static int mkv_write_attachments(AVFormatContext *s) { MatroskaMuxContext *mkv = s->priv_data; -AVIOContext *pb = s->pb; +AVIOContext *dyn_cp, *pb = s->pb; ebml_master attachments; AVLFG c; int i, ret; @@ -1480,7 +1480,8 @@ static int mkv_write_attachments(AVFormatContext *s) ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_ATTACHMENTS, avio_tell(pb)); if (ret < 0) return ret; -attachments = start_ebml_master(pb, MATROSKA_ID_ATTACHMENTS, 0); +ret = start_ebml_master_crc32(pb, &dyn_cp, &attachments, MATROSKA_ID_ATTACHMENTS, 0); +if (ret < 0) return ret; for (i = 0; i < s->nb_streams; i++) { AVStream *st = s->streams[i]; @@ -1492,15 +1493,15 @@ static int mkv_write_attachments(AVFormatContext *s) if (st->codecpar->codec_type != AVMEDIA_TYPE_ATTACHMENT) continue; -attached_file = start_ebml_master(pb, MATROSKA_ID_ATTACHEDFILE, 0); +attached_file = start_ebml_master(dyn_cp, MATROSKA_ID_ATTACHEDFILE, 0); if (t = av_dict_get(st->metadata, "title", NULL, 0)) -put_ebml_string(pb, MATROSKA_ID_FILEDESC, t->value); +put_ebml_string(dyn_cp, MATROSKA_ID_FILEDESC, t->value); if (!(t = av_dict_get(st->metadata, "filename", NULL, 0))) { av_log(s, AV_LOG_ERROR, "Attachment stream %d has no filename tag.\n", i); return AVERROR(EINVAL); } -put_ebml_string(pb, MATROSKA_ID_FILENAME, t->value); +put_ebml_string(dyn_cp, MATROSKA_ID_FILENAME, t->value); if (t = av_dict_get(st->metadata, "mimetype", NULL, 0)) mimetype = t->value; else if (st->codecpar->codec_id != AV_CODEC_ID_NONE ) { @@ -1538,12 +1539,12 @@ static int mkv_write_attachments(AVFormatContext *s) av_log(s, AV_LOG_VERBOSE, "Using %.16"PRIx64" for attachment %d\n", fileuid, i); -put_ebml_string(pb, MATROSKA_ID_FILEMIMETYPE, mimetype); -put_ebml_binary(pb, MATROSKA_ID_FILEDATA, st->codecpar->extradata, st->codecpar->extradata_size); -put_ebml_uint(pb, MATROSKA_ID_FILEUID, fileuid); -end_ebml_master(pb, attached_file); +put_ebml_string(dyn_cp, MATROSKA_ID_FILEMIMETYPE, mimetype); +put_ebml_binary(dyn_cp, MATROSKA_ID_FILEDATA, st->codecpar->extradata, st->codecpar->extradata_size); +put_ebml_uint(dyn_cp, MATROSKA_ID_FILEUID, fileuid); +end_ebml_master(dyn_cp, attached_file); } -end_ebml_master(pb, attachments); +end_ebml_master_crc32(pb, &dyn_cp, mkv, attachments); return 0; } diff --git a/tests/ref/lavf/mkv b/tests/ref/lavf/mkv index a0ba445..951feac 100644 --- a/tests/ref/lavf/mkv +++ b/tests/ref/lavf/mkv @@ -1,5 +1,5 @@ -ef45f99b01ec39e3a8e744fbde78 *./tests/data/lavf/lavf.mkv -472932 ./tests/data/lavf/lavf.mkv +b3599e3229821a84116b4f03f324a08b *./tests/data/lavf/lavf.mkv +472938 ./tests/data/lavf/lavf.mkv ./tests/data/lavf/lavf.mkv CRC=0xec6c3c68 c56b90945e6e14a9b4b7f1ab94e3ad28 *./tests/data/lavf/lavf.mkv 320608 ./tests/data/lavf/lavf.mkv -- 2.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 07/11] avformat/matroskaenc: write a CRC32 element on Tracks
Implements part of ticket #4347 Signed-off-by: James Almer --- libavformat/matroskaenc.c | 11 +++ tests/ref/fate/rgb24-mkv | 4 ++-- tests/ref/lavf/mka| 4 ++-- tests/ref/lavf/mkv| 8 tests/ref/seek/lavf-mkv | 44 ++-- 5 files changed, 37 insertions(+), 34 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 56ee1ec..d2b158f 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1226,7 +1226,7 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv, static int mkv_write_tracks(AVFormatContext *s) { MatroskaMuxContext *mkv = s->priv_data; -AVIOContext *pb = s->pb; +AVIOContext *dyn_cp, *pb = s->pb; ebml_master tracks; int i, ret, default_stream_exists = 0; @@ -1234,17 +1234,20 @@ static int mkv_write_tracks(AVFormatContext *s) if (ret < 0) return ret; -tracks = start_ebml_master(pb, MATROSKA_ID_TRACKS, 0); +ret = start_ebml_master_crc32(pb, &dyn_cp, &tracks, MATROSKA_ID_TRACKS, 0); +if (ret < 0) +return ret; + for (i = 0; i < s->nb_streams; i++) { AVStream *st = s->streams[i]; default_stream_exists |= st->disposition & AV_DISPOSITION_DEFAULT; } for (i = 0; i < s->nb_streams; i++) { -ret = mkv_write_track(s, mkv, i, pb, default_stream_exists); +ret = mkv_write_track(s, mkv, i, dyn_cp, default_stream_exists); if (ret < 0) return ret; } -end_ebml_master(pb, tracks); +end_ebml_master_crc32(pb, &dyn_cp, mkv, tracks); return 0; } diff --git a/tests/ref/fate/rgb24-mkv b/tests/ref/fate/rgb24-mkv index d1a9b6d..01b069c 100644 --- a/tests/ref/fate/rgb24-mkv +++ b/tests/ref/fate/rgb24-mkv @@ -1,5 +1,5 @@ -7f800fb3147ca8441f6b4d8cdfeec6d5 *tests/data/fate/rgb24-mkv.matroska -58346 tests/data/fate/rgb24-mkv.matroska +f2ec884f82ecf5754afc0c9a2babe4aa *tests/data/fate/rgb24-mkv.matroska +58352 tests/data/fate/rgb24-mkv.matroska #tb 0: 1/10 #media_type 0: video #codec_id 0: rawvideo diff --git a/tests/ref/lavf/mka b/tests/ref/lavf/mka index 3852d28..1990715 100644 --- a/tests/ref/lavf/mka +++ b/tests/ref/lavf/mka @@ -1,3 +1,3 @@ -57d2f97a7c4d13c97212cb0f76f72e10 *./tests/data/lavf/lavf.mka -43666 ./tests/data/lavf/lavf.mka +927a5d1e7837735271f57b329f1c9d7a *./tests/data/lavf/lavf.mka +43672 ./tests/data/lavf/lavf.mka ./tests/data/lavf/lavf.mka CRC=0x3a1da17e diff --git a/tests/ref/lavf/mkv b/tests/ref/lavf/mkv index 664a84d..a0ba445 100644 --- a/tests/ref/lavf/mkv +++ b/tests/ref/lavf/mkv @@ -1,6 +1,6 @@ -d0376acc0f57613e193192a685d9a4d6 *./tests/data/lavf/lavf.mkv -472926 ./tests/data/lavf/lavf.mkv +ef45f99b01ec39e3a8e744fbde78 *./tests/data/lavf/lavf.mkv +472932 ./tests/data/lavf/lavf.mkv ./tests/data/lavf/lavf.mkv CRC=0xec6c3c68 -a08252617efbd7de6c8262e13ca7f3e4 *./tests/data/lavf/lavf.mkv -320602 ./tests/data/lavf/lavf.mkv +c56b90945e6e14a9b4b7f1ab94e3ad28 *./tests/data/lavf/lavf.mkv +320608 ./tests/data/lavf/lavf.mkv ./tests/data/lavf/lavf.mkv CRC=0xec6c3c68 diff --git a/tests/ref/seek/lavf-mkv b/tests/ref/seek/lavf-mkv index 0662f2f..7bd1586 100644 --- a/tests/ref/seek/lavf-mkv +++ b/tests/ref/seek/lavf-mkv @@ -1,48 +1,48 @@ -ret: 0 st: 1 flags:1 dts: 0.00 pts: 0.00 pos:812 size: 208 +ret: 0 st: 1 flags:1 dts: 0.00 pts: 0.00 pos:818 size: 208 ret: 0 st:-1 flags:0 ts:-1.00 -ret: 0 st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos: 1028 size: 27837 +ret: 0 st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos: 1034 size: 27837 ret: 0 st:-1 flags:1 ts: 1.894167 -ret: 0 st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292454 size: 27834 +ret: 0 st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292460 size: 27834 ret: 0 st: 0 flags:0 ts: 0.788000 -ret: 0 st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292454 size: 27834 +ret: 0 st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292460 size: 27834 ret: 0 st: 0 flags:1 ts:-0.317000 -ret: 0 st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos: 1028 size: 27837 +ret: 0 st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos: 1034 size: 27837 ret:-1 st: 1 flags:0 ts: 2.577000 ret: 0 st: 1 flags:1 ts: 1.471000 -ret: 0 st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320295 size: 209 +ret: 0 st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320301 size: 209 ret: 0 st:-1 flags:0 ts: 0.365002 -ret: 0 st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 147001 size: 27925 +ret: 0 st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 147007 size: 27925 ret: 0 st:-1 flags:1 ts:-0.740831 -ret: 0 st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos: 1028 size: 27837 +ret: 0 st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos: 1034 size: 27837 ret:-1 st: 0 flags:0
[FFmpeg-devel] [PATCH 08/11] avformat/matroskaenc: write a CRC32 element on Chapters
Implements part of ticket #4347 Signed-off-by: James Almer --- libavformat/matroskaenc.c | 38 -- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index d2b158f..9687833 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1254,7 +1254,7 @@ static int mkv_write_tracks(AVFormatContext *s) static int mkv_write_chapters(AVFormatContext *s) { MatroskaMuxContext *mkv = s->priv_data; -AVIOContext *pb = s->pb; +AVIOContext *dyn_cp, *pb = s->pb; ebml_master chapters, editionentry; AVRational scale = {1, 1E9}; int i, ret; @@ -1265,10 +1265,12 @@ static int mkv_write_chapters(AVFormatContext *s) ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_CHAPTERS, avio_tell(pb)); if (ret < 0) return ret; -chapters = start_ebml_master(pb, MATROSKA_ID_CHAPTERS, 0); -editionentry = start_ebml_master(pb, MATROSKA_ID_EDITIONENTRY, 0); -put_ebml_uint(pb, MATROSKA_ID_EDITIONFLAGDEFAULT, 1); -put_ebml_uint(pb, MATROSKA_ID_EDITIONFLAGHIDDEN , 0); +ret = start_ebml_master_crc32(pb, &dyn_cp, &chapters, MATROSKA_ID_CHAPTERS, 0); +if (ret < 0) return ret; + +editionentry = start_ebml_master(dyn_cp, MATROSKA_ID_EDITIONENTRY, 0); +put_ebml_uint(dyn_cp, MATROSKA_ID_EDITIONFLAGDEFAULT, 1); +put_ebml_uint(dyn_cp, MATROSKA_ID_EDITIONFLAGHIDDEN , 0); for (i = 0; i < s->nb_chapters; i++) { ebml_master chapteratom, chapterdisplay; AVChapter *c = s->chapters[i]; @@ -1282,22 +1284,22 @@ static int mkv_write_chapters(AVFormatContext *s) return AVERROR_INVALIDDATA; } -chapteratom = start_ebml_master(pb, MATROSKA_ID_CHAPTERATOM, 0); -put_ebml_uint(pb, MATROSKA_ID_CHAPTERUID, c->id + mkv->chapter_id_offset); -put_ebml_uint(pb, MATROSKA_ID_CHAPTERTIMESTART, chapterstart); -put_ebml_uint(pb, MATROSKA_ID_CHAPTERTIMEEND, chapterend); -put_ebml_uint(pb, MATROSKA_ID_CHAPTERFLAGHIDDEN , 0); -put_ebml_uint(pb, MATROSKA_ID_CHAPTERFLAGENABLED, 1); +chapteratom = start_ebml_master(dyn_cp, MATROSKA_ID_CHAPTERATOM, 0); +put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERUID, c->id + mkv->chapter_id_offset); +put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMESTART, chapterstart); +put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMEEND, chapterend); +put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERFLAGHIDDEN , 0); +put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERFLAGENABLED, 1); if ((t = av_dict_get(c->metadata, "title", NULL, 0))) { -chapterdisplay = start_ebml_master(pb, MATROSKA_ID_CHAPTERDISPLAY, 0); -put_ebml_string(pb, MATROSKA_ID_CHAPSTRING, t->value); -put_ebml_string(pb, MATROSKA_ID_CHAPLANG , "und"); -end_ebml_master(pb, chapterdisplay); +chapterdisplay = start_ebml_master(dyn_cp, MATROSKA_ID_CHAPTERDISPLAY, 0); +put_ebml_string(dyn_cp, MATROSKA_ID_CHAPSTRING, t->value); +put_ebml_string(dyn_cp, MATROSKA_ID_CHAPLANG , "und"); +end_ebml_master(dyn_cp, chapterdisplay); } -end_ebml_master(pb, chapteratom); +end_ebml_master(dyn_cp, chapteratom); } -end_ebml_master(pb, editionentry); -end_ebml_master(pb, chapters); +end_ebml_master(dyn_cp, editionentry); +end_ebml_master_crc32(pb, &dyn_cp, mkv, chapters); mkv->wrote_chapters = 1; return 0; -- 2.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 10/11] avformat/matroskaenc: write a CRC32 element on Tags
Implements part of ticket #4347 Signed-off-by: James Almer --- This one got messy because we update Duration tags at the end of the muxing process, and the entire master needs to be finalized before the CRC32 can be calculated. libavformat/matroskaenc.c | 58 +-- tests/ref/fate/rgb24-mkv | 4 ++-- tests/ref/lavf/mka| 4 ++-- tests/ref/lavf/mkv| 8 +++ tests/ref/seek/lavf-mkv | 44 +-- 5 files changed, 66 insertions(+), 52 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index c63ecdd..4dfcc55 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -107,6 +107,8 @@ typedef struct MatroskaMuxContext { const AVClass *class; int mode; AVIOContext *dyn_bc; +AVIOContext *tags_bc; +ebml_master tags; ebml_master segment; int64_t segment_offset; ebml_master cluster; @@ -1343,6 +1345,7 @@ static int mkv_write_tag_targets(AVFormatContext *s, unsigned int elementid, unsigned int uid, ebml_master *tags, ebml_master* tag) { +AVIOContext *pb; MatroskaMuxContext *mkv = s->priv_data; ebml_master targets; int ret; @@ -1351,14 +1354,15 @@ static int mkv_write_tag_targets(AVFormatContext *s, ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_TAGS, avio_tell(s->pb)); if (ret < 0) return ret; -*tags = start_ebml_master(s->pb, MATROSKA_ID_TAGS, 0); +start_ebml_master_crc32(s->pb, &mkv->tags_bc, tags, MATROSKA_ID_TAGS, 0); } +pb = mkv->tags_bc; -*tag = start_ebml_master(s->pb, MATROSKA_ID_TAG,0); -targets = start_ebml_master(s->pb, MATROSKA_ID_TAGTARGETS, 0); +*tag = start_ebml_master(pb, MATROSKA_ID_TAG, 0); +targets = start_ebml_master(pb, MATROSKA_ID_TAGTARGETS, 0); if (elementid) -put_ebml_uint(s->pb, elementid, uid); -end_ebml_master(s->pb, targets); +put_ebml_uint(pb, elementid, uid); +end_ebml_master(pb, targets); return 0; } @@ -1376,6 +1380,7 @@ static int mkv_check_tag_name(const char *name, unsigned int elementid) static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, unsigned int elementid, unsigned int uid, ebml_master *tags) { +MatroskaMuxContext *mkv = s->priv_data; ebml_master tag; int ret; AVDictionaryEntry *t = NULL; @@ -1386,13 +1391,13 @@ static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, unsigned int eleme while ((t = av_dict_get(m, "", t, AV_DICT_IGNORE_SUFFIX))) { if (mkv_check_tag_name(t->key, elementid)) { -ret = mkv_write_simpletag(s->pb, t); +ret = mkv_write_simpletag(mkv->tags_bc, t); if (ret < 0) return ret; } } -end_ebml_master(s->pb, tag); +end_ebml_master(mkv->tags_bc, tag); return 0; } @@ -1410,13 +1415,12 @@ static int mkv_check_tag(AVDictionary *m, unsigned int elementid) static int mkv_write_tags(AVFormatContext *s) { MatroskaMuxContext *mkv = s->priv_data; -ebml_master tags = {0}; int i, ret; ff_metadata_conv_ctx(s, ff_mkv_metadata_conv, NULL); if (mkv_check_tag(s->metadata, 0)) { -ret = mkv_write_tag(s, s->metadata, 0, 0, &tags); +ret = mkv_write_tag(s, s->metadata, 0, 0, &mkv->tags); if (ret < 0) return ret; } @@ -1426,26 +1430,28 @@ static int mkv_write_tags(AVFormatContext *s) if (!mkv_check_tag(st->metadata, MATROSKA_ID_TAGTARGETS_TRACKUID)) continue; -ret = mkv_write_tag(s, st->metadata, MATROSKA_ID_TAGTARGETS_TRACKUID, i + 1, &tags); +ret = mkv_write_tag(s, st->metadata, MATROSKA_ID_TAGTARGETS_TRACKUID, i + 1, &mkv->tags); if (ret < 0) return ret; } if (s->pb->seekable && !mkv->is_live) { for (i = 0; i < s->nb_streams; i++) { +AVIOContext *pb; ebml_master tag_target; ebml_master tag; -mkv_write_tag_targets(s, MATROSKA_ID_TAGTARGETS_TRACKUID, i + 1, &tags, &tag_target); +mkv_write_tag_targets(s, MATROSKA_ID_TAGTARGETS_TRACKUID, i + 1, &mkv->tags, &tag_target); +pb = mkv->tags_bc; -tag = start_ebml_master(s->pb, MATROSKA_ID_SIMPLETAG, 0); -put_ebml_string(s->pb, MATROSKA_ID_TAGNAME, "DURATION"); -mkv->stream_duration_offsets[i] = avio_tell(s->pb); +tag = start_ebml_master(pb, MATROSKA_ID_SIMPLETAG, 0); +put_ebml_string(pb, MATROSKA_ID_TAGNAME, "DURATION"); +mkv->stream_duration_offsets[i] = avio_tell(pb); // Reserve space to write duration as a 20-byte string. // 2 (ebml id) + 1 (data size) + 20 (data) -put_ebml_void(s->pb, 23); -end_ebml_mast
[FFmpeg-devel] [PATCH 06/11] avformat/matroskaenc: write a CRC32 element on Cues
Implements part of ticket #4347 Signed-off-by: James Almer --- libavformat/matroskaenc.c | 29 - tests/ref/fate/rgb24-mkv | 4 ++-- tests/ref/lavf/mkv| 8 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index da45f7c..56ee1ec 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -514,13 +514,16 @@ static int mkv_add_cuepoint(mkv_cues *cues, int stream, int tracknum, int64_t ts static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tracks, int num_tracks) { -AVIOContext *pb = s->pb; +MatroskaMuxContext *mkv = s->priv_data; +AVIOContext *dyn_cp, *pb = s->pb; ebml_master cues_element; int64_t currentpos; -int i, j; +int i, j, ret; currentpos = avio_tell(pb); -cues_element = start_ebml_master(pb, MATROSKA_ID_CUES, 0); +ret = start_ebml_master_crc32(pb, &dyn_cp, &cues_element, MATROSKA_ID_CUES, 0); +if (ret < 0) +return ret; for (i = 0; i < cues->num_entries; i++) { ebml_master cuepoint, track_positions; @@ -540,8 +543,8 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra ctp_nb ++; } -cuepoint = start_ebml_master(pb, MATROSKA_ID_POINTENTRY, MAX_CUEPOINT_SIZE(ctp_nb)); -put_ebml_uint(pb, MATROSKA_ID_CUETIME, pts); +cuepoint = start_ebml_master(dyn_cp, MATROSKA_ID_POINTENTRY, MAX_CUEPOINT_SIZE(ctp_nb)); +put_ebml_uint(dyn_cp, MATROSKA_ID_CUETIME, pts); // put all the entries from different tracks that have the exact same // timestamp into the same CuePoint @@ -553,18 +556,18 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra if (tracks[tracknum].has_cue && s->streams[tracknum]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE) continue; tracks[tracknum].has_cue = 1; -track_positions = start_ebml_master(pb, MATROSKA_ID_CUETRACKPOSITION, MAX_CUETRACKPOS_SIZE); -put_ebml_uint(pb, MATROSKA_ID_CUETRACK , entry[j].tracknum ); -put_ebml_uint(pb, MATROSKA_ID_CUECLUSTERPOSITION , entry[j].cluster_pos); -put_ebml_uint(pb, MATROSKA_ID_CUERELATIVEPOSITION, entry[j].relative_pos); +track_positions = start_ebml_master(dyn_cp, MATROSKA_ID_CUETRACKPOSITION, MAX_CUETRACKPOS_SIZE); +put_ebml_uint(dyn_cp, MATROSKA_ID_CUETRACK , entry[j].tracknum ); +put_ebml_uint(dyn_cp, MATROSKA_ID_CUECLUSTERPOSITION , entry[j].cluster_pos); +put_ebml_uint(dyn_cp, MATROSKA_ID_CUERELATIVEPOSITION, entry[j].relative_pos); if (entry[j].duration != -1) -put_ebml_uint(pb, MATROSKA_ID_CUEDURATION, entry[j].duration); -end_ebml_master(pb, track_positions); +put_ebml_uint(dyn_cp, MATROSKA_ID_CUEDURATION, entry[j].duration); +end_ebml_master(dyn_cp, track_positions); } i += j - 1; -end_ebml_master(pb, cuepoint); +end_ebml_master(dyn_cp, cuepoint); } -end_ebml_master(pb, cues_element); +end_ebml_master_crc32(pb, &dyn_cp, mkv, cues_element); return currentpos; } diff --git a/tests/ref/fate/rgb24-mkv b/tests/ref/fate/rgb24-mkv index 6bcffa1..d1a9b6d 100644 --- a/tests/ref/fate/rgb24-mkv +++ b/tests/ref/fate/rgb24-mkv @@ -1,5 +1,5 @@ -c368e77c46109ac20e279ff83f9649b3 *tests/data/fate/rgb24-mkv.matroska -58340 tests/data/fate/rgb24-mkv.matroska +7f800fb3147ca8441f6b4d8cdfeec6d5 *tests/data/fate/rgb24-mkv.matroska +58346 tests/data/fate/rgb24-mkv.matroska #tb 0: 1/10 #media_type 0: video #codec_id 0: rawvideo diff --git a/tests/ref/lavf/mkv b/tests/ref/lavf/mkv index 9c9394f..664a84d 100644 --- a/tests/ref/lavf/mkv +++ b/tests/ref/lavf/mkv @@ -1,6 +1,6 @@ -161e0ed817bd65194fd4001c0d5380f0 *./tests/data/lavf/lavf.mkv -472920 ./tests/data/lavf/lavf.mkv +d0376acc0f57613e193192a685d9a4d6 *./tests/data/lavf/lavf.mkv +472926 ./tests/data/lavf/lavf.mkv ./tests/data/lavf/lavf.mkv CRC=0xec6c3c68 -7afba147b92da5115095fa52e214cfee *./tests/data/lavf/lavf.mkv -320596 ./tests/data/lavf/lavf.mkv +a08252617efbd7de6c8262e13ca7f3e4 *./tests/data/lavf/lavf.mkv +320602 ./tests/data/lavf/lavf.mkv ./tests/data/lavf/lavf.mkv CRC=0xec6c3c68 -- 2.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 11/11] avformat/matroskaenc: write a CRC32 element on Info
Finishes implementing ticket #4347 Signed-off-by: James Almer --- Same situation as Tags, the Duration is updated at the end of the muxing process. libavformat/matroskaenc.c | 22 +- tests/ref/fate/rgb24-mkv | 4 ++-- tests/ref/lavf/mka| 4 ++-- tests/ref/lavf/mkv| 8 tests/ref/seek/lavf-mkv | 44 ++-- 5 files changed, 47 insertions(+), 35 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 4dfcc55..fc8d84e 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -109,6 +109,8 @@ typedef struct MatroskaMuxContext { AVIOContext *dyn_bc; AVIOContext *tags_bc; ebml_master tags; +AVIOContext *info_bc; +ebml_master info; ebml_master segment; int64_t segment_offset; ebml_master cluster; @@ -1587,7 +1589,7 @@ static int mkv_write_header(AVFormatContext *s) { MatroskaMuxContext *mkv = s->priv_data; AVIOContext *pb = s->pb; -ebml_master ebml_header, segment_info; +ebml_master ebml_header; AVDictionaryEntry *tag; int ret, i, version = 2; int64_t creation_time; @@ -1652,7 +1654,11 @@ static int mkv_write_header(AVFormatContext *s) ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_INFO, avio_tell(pb)); if (ret < 0) goto fail; -segment_info = start_ebml_master(pb, MATROSKA_ID_INFO, 0); +ret = start_ebml_master_crc32(pb, &mkv->info_bc, &mkv->info, MATROSKA_ID_INFO, 0); +if (ret < 0) +return ret; +pb = mkv->info_bc; + put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 100); if ((tag = av_dict_get(s->metadata, "title", NULL, 0))) put_ebml_string(pb, MATROSKA_ID_TITLE, tag->value); @@ -1706,7 +1712,11 @@ static int mkv_write_header(AVFormatContext *s) put_ebml_void(pb, 11); // assumes double-precision float to be written } } -end_ebml_master(pb, segment_info); +if (s->pb->seekable) +put_ebml_void(s->pb, avio_tell(pb) + ((mkv->mode != MODE_WEBM) ? 2 /* ebml id + data size */ + 4 /* CRC32 */ : 0)); +else +end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, mkv->info); +pb = s->pb; // initialize stream_duration fields mkv->stream_durations = av_mallocz(s->nb_streams * sizeof(int64_t)); @@ -2231,8 +2241,10 @@ static int mkv_write_trailer(AVFormatContext *s) // update the duration av_log(s, AV_LOG_DEBUG, "end duration = %" PRIu64 "\n", mkv->duration); currentpos = avio_tell(pb); -avio_seek(pb, mkv->duration_offset, SEEK_SET); -put_ebml_float(pb, MATROSKA_ID_DURATION, mkv->duration); +avio_seek(mkv->info_bc, mkv->duration_offset, SEEK_SET); +put_ebml_float(mkv->info_bc, MATROSKA_ID_DURATION, mkv->duration); +avio_seek(pb, mkv->info.pos, SEEK_SET); +end_ebml_master_crc32(pb, &mkv->info_bc, mkv, mkv->info); // update stream durations if (mkv->stream_durations) { diff --git a/tests/ref/fate/rgb24-mkv b/tests/ref/fate/rgb24-mkv index 3cd1887..9e19123 100644 --- a/tests/ref/fate/rgb24-mkv +++ b/tests/ref/fate/rgb24-mkv @@ -1,5 +1,5 @@ -cddd0f9c0efc6592bd3026b8c47471c3 *tests/data/fate/rgb24-mkv.matroska -58358 tests/data/fate/rgb24-mkv.matroska +af0473f323c466461a73f96ea7955ddd *tests/data/fate/rgb24-mkv.matroska +58364 tests/data/fate/rgb24-mkv.matroska #tb 0: 1/10 #media_type 0: video #codec_id 0: rawvideo diff --git a/tests/ref/lavf/mka b/tests/ref/lavf/mka index cde5cf9..ac0f6cf 100644 --- a/tests/ref/lavf/mka +++ b/tests/ref/lavf/mka @@ -1,3 +1,3 @@ -afd0c76b5fd8ca5ee47d12af7f92d024 *./tests/data/lavf/lavf.mka -43678 ./tests/data/lavf/lavf.mka +2d9722c0691e140237af0036e3a178b0 *./tests/data/lavf/lavf.mka +43684 ./tests/data/lavf/lavf.mka ./tests/data/lavf/lavf.mka CRC=0x3a1da17e diff --git a/tests/ref/lavf/mkv b/tests/ref/lavf/mkv index 63ed7da..aa60115 100644 --- a/tests/ref/lavf/mkv +++ b/tests/ref/lavf/mkv @@ -1,6 +1,6 @@ -aa0fabb3a1564adbdbd27310b1643ca3 *./tests/data/lavf/lavf.mkv -472944 ./tests/data/lavf/lavf.mkv +12f692ca7c40fede8be4c14f375ddf9a *./tests/data/lavf/lavf.mkv +472950 ./tests/data/lavf/lavf.mkv ./tests/data/lavf/lavf.mkv CRC=0xec6c3c68 -85f86c9d5641c2344b9b389f38fad890 *./tests/data/lavf/lavf.mkv -320614 ./tests/data/lavf/lavf.mkv +1699857c959bceff0063e7b123b35969 *./tests/data/lavf/lavf.mkv +320620 ./tests/data/lavf/lavf.mkv ./tests/data/lavf/lavf.mkv CRC=0xec6c3c68 diff --git a/tests/ref/seek/lavf-mkv b/tests/ref/seek/lavf-mkv index 26df545..4bb26ef 100644 --- a/tests/ref/seek/lavf-mkv +++ b/tests/ref/seek/lavf-mkv @@ -1,48 +1,48 @@ -ret: 0 st: 1 flags:1 dts: 0.00 pts: 0.00 pos:824 size: 208 +ret: 0 st: 1 flags:1 dts: 0.00 pts: 0.00 pos:830 size: 208 ret: 0 st:-1 flags:0 ts:-1.00 -ret: 0 st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos: 1040 size
[FFmpeg-devel] [PATCH] avformat/matroskaenc: fix targets for attachment tags
Attachment tags were being written targetting non-existent streams in the output file. Also filter "filename" and "mimetype" entries, as they are standard elements in the Attachment master. Signed-off-by: James Almer --- The fileuid is changed to be four bytes long instead of eight, because the mkv_write_tag*() functions pass that value as unsigned int. If preferred i can change them to pass uint64_t and keep the current huge fileuids. libavformat/matroskaenc.c | 66 ++- tests/ref/lavf/mkv| 4 +-- 2 files changed, 61 insertions(+), 9 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 3eeb09b..4561db7 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -95,6 +95,16 @@ typedef struct mkv_track { int64_t ts_offset; } mkv_track; +typedef struct mkv_attachment { +int stream_idx; +uint32_tfileuid; +} mkv_attachment; + +typedef struct mkv_attachments { +mkv_attachment *entries; +int num_entries; +} mkv_attachments; + #define MODE_MATROSKAv2 0x01 #define MODE_WEBM 0x02 @@ -116,6 +126,7 @@ typedef struct MatroskaMuxContext { mkv_seekhead*main_seekhead; mkv_cues*cues; mkv_track *tracks; +mkv_attachments *attachments; AVPacketcur_audio_pkt; @@ -323,6 +334,10 @@ static void mkv_free(MatroskaMuxContext *mkv) { av_freep(&mkv->cues->entries); av_freep(&mkv->cues); } +if (mkv->attachments) { +av_freep(&mkv->attachments->entries); +av_freep(&mkv->attachments); +} av_freep(&mkv->tracks); av_freep(&mkv->stream_durations); av_freep(&mkv->stream_duration_offsets); @@ -1316,7 +1331,10 @@ static int mkv_check_tag_name(const char *name, unsigned int elementid) av_strcasecmp(name, "encoding_tool") && av_strcasecmp(name, "duration") && (elementid != MATROSKA_ID_TAGTARGETS_TRACKUID || -av_strcasecmp(name, "language")); +av_strcasecmp(name, "language")) && + (elementid != MATROSKA_ID_TAGTARGETS_ATTACHUID || +(av_strcasecmp(name, "filename") && + av_strcasecmp(name, "mimetype"))); } static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, unsigned int elementid, @@ -1369,6 +1387,9 @@ static int mkv_write_tags(AVFormatContext *s) for (i = 0; i < s->nb_streams; i++) { AVStream *st = s->streams[i]; +if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT) +continue; + if (!mkv_check_tag(st->metadata, MATROSKA_ID_TAGTARGETS_TRACKUID)) continue; @@ -1378,9 +1399,13 @@ static int mkv_write_tags(AVFormatContext *s) if (!mkv->is_live) { for (i = 0; i < s->nb_streams; i++) { +AVStream *st = s->streams[i]; ebml_master tag_target; ebml_master tag; +if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT) +continue; + mkv_write_tag_targets(s, MATROSKA_ID_TAGTARGETS_TRACKUID, i + 1, &tags, &tag_target); tag = start_ebml_master(s->pb, MATROSKA_ID_SIMPLETAG, 0); @@ -1405,6 +1430,20 @@ static int mkv_write_tags(AVFormatContext *s) if (ret < 0) return ret; } +if (mkv->have_attachments) { +for (i = 0; i < mkv->attachments->num_entries; i++) { +mkv_attachment *attachment = &mkv->attachments->entries[i]; +AVStream *st = s->streams[attachment->stream_idx]; + +if (!mkv_check_tag(st->metadata, MATROSKA_ID_TAGTARGETS_ATTACHUID)) +continue; + +ret = mkv_write_tag(s, st->metadata, MATROSKA_ID_TAGTARGETS_ATTACHUID, attachment->fileuid, &tags); +if (ret < 0) +return ret; +} +} + if (tags.pos) end_ebml_master(s->pb, tags); return 0; @@ -1421,6 +1460,10 @@ static int mkv_write_attachments(AVFormatContext *s) if (!mkv->have_attachments) return 0; +mkv->attachments = av_mallocz(sizeof(*mkv->attachments)); +if (!mkv->attachments) +return ret; + av_lfg_init(&c, av_get_random_seed()); ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_ATTACHMENTS, avio_tell(pb)); @@ -1431,13 +1474,19 @@ static int mkv_write_attachments(AVFormatContext *s) for (i = 0; i < s->nb_streams; i++) { AVStream *st = s->streams[i]; ebml_master attached_file; +mkv_attachment *attachment = mkv->attachments->entries; AVDictionaryEntry *t; const char *mimetype = NULL; -uint64_t fileuid; +uint32_t fileuid; if (st->codecpar->codec_type != AVMEDIA_TYPE_ATTACHMENT) continue; +attachment = av_realloc_array(attachment, mkv->attachments->num_entries + 1, sizeof(mkv_attachment)); +if (!attachment) +return AVERROR(
Re: [FFmpeg-devel] [PATCH 2/4] V12 - SCTE-35 extraction from mpegts
On Sat, Oct 1, 2016 at 10:50 AM, Marton Balint wrote: > > Empty line. Thanks, corrected in V13. If all OK, can this patchset be merged? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] lavc/libmp3lame: send encoder padding in side data of final packet
--- libavcodec/libmp3lame.c | 21 - 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/libavcodec/libmp3lame.c b/libavcodec/libmp3lame.c index 5642264..a1bf122 100644 --- a/libavcodec/libmp3lame.c +++ b/libavcodec/libmp3lame.c @@ -185,7 +185,7 @@ static int mp3lame_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, { LAMEContext *s = avctx->priv_data; MPADecodeHeader hdr; -int len, ret, ch; +int len, ret, ch, discard_padding; int lame_result; uint32_t h; @@ -269,6 +269,25 @@ static int mp3lame_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, ff_af_queue_remove(&s->afq, avctx->frame_size, &avpkt->pts, &avpkt->duration); +discard_padding = avctx->frame_size - avpkt->duration; +// Check if subtraction resulted in an overflow +if ((discard_padding < avctx->frame_size) != (avpkt->duration > 0)) { +av_packet_unref(avpkt); +av_free(avpkt); +return AVERROR(EINVAL); +} +if (discard_padding > 0) { +uint8_t* side_data = av_packet_new_side_data(avpkt, + AV_PKT_DATA_SKIP_SAMPLES, + 10); +if(!side_data) { +av_packet_unref(avpkt); +av_free(avpkt); +return AVERROR(ENOMEM); +} +AV_WL32(side_data + 4, discard_padding); +} + avpkt->size = len; *got_packet_ptr = 1; } -- 2.8.0.rc3.226.g39d4020 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/3] lavf/mp3enc: write encoder delay/padding upon closing
--- libavformat/mp3enc.c | 32 +--- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c index de63401..48cb0b4 100644 --- a/libavformat/mp3enc.c +++ b/libavformat/mp3enc.c @@ -111,6 +111,7 @@ typedef struct MP3Context { uint64_t bag[XING_NUM_BAGS]; int initial_bitrate; int has_variable_bitrate; +int trailing_padding; /* index of the audio stream */ int audio_stream_idx; @@ -247,12 +248,7 @@ static int mp3_write_xing(AVFormatContext *s) ffio_fill(dyn_ctx, 0, 8); // empty replaygain fields avio_w8(dyn_ctx, 0); // unknown encoding flags avio_w8(dyn_ctx, 0); // unknown abr/minimal bitrate - -// encoder delay -if (par->initial_padding - 528 - 1 >= 1 << 12) { -av_log(s, AV_LOG_WARNING, "Too many samples of initial padding.\n"); -} -avio_wb24(dyn_ctx, FFMAX(par->initial_padding - 528 - 1, 0)<<12); +avio_wb24(dyn_ctx, 0);// empty encoder delay/padding avio_w8(dyn_ctx, 0); // misc avio_w8(dyn_ctx, 0); // mp3gain @@ -345,10 +341,22 @@ static int mp3_write_audio_packet(AVFormatContext *s, AVPacket *pkt) #endif if (mp3->xing_offset) { +uint8_t *side_data = NULL; +int side_data_size = 0; + mp3_xing_add_frame(mp3, pkt); mp3->audio_size += pkt->size; mp3->audio_crc = av_crc(av_crc_get_table(AV_CRC_16_ANSI_LE), mp3->audio_crc, pkt->data, pkt->size); + +side_data = av_packet_get_side_data(pkt, +AV_PKT_DATA_SKIP_SAMPLES, +&side_data_size); +if (side_data && side_data_size >= 10) { +mp3->trailing_padding = AV_RL32(side_data + 4); +} else { +mp3->trailing_padding = 0; +} } } @@ -381,7 +389,7 @@ static void mp3_update_xing(AVFormatContext *s) AVReplayGain *rg; uint16_t tag_crc; uint8_t *toc; -int i, rg_size; +int i, rg_size, delay; /* replace "Xing" identification string with "Info" for CBR files. */ if (!mp3->has_variable_bitrate) @@ -422,6 +430,16 @@ static void mp3_update_xing(AVFormatContext *s) } } +/* write encoder delay/padding */ +delay = FFMAX(s->streams[0]->codecpar->initial_padding - 528 - 1, 0); +if (delay >= 1 << 12) { +av_log(s, AV_LOG_WARNING, "Too many samples of initial padding.\n"); +} +if (mp3->trailing_padding >= 1 << 12) { +av_log(s, AV_LOG_WARNING, "Too many samples of trailing padding.\n"); +} +AV_WB24(mp3->xing_frame + mp3->xing_offset + 141, (delay << 12) + mp3->trailing_padding); + AV_WB32(mp3->xing_frame + mp3->xing_offset + XING_SIZE - 8, mp3->audio_size); AV_WB16(mp3->xing_frame + mp3->xing_offset + XING_SIZE - 4, mp3->audio_crc); -- 2.8.0.rc3.226.g39d4020 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 0/3] Fix gapless encoding/decoding for MP3
Round trip wav->mp3->wav now preserves the correct number of samples. Remuxing mp3->mp3 with -c:a copy also preserves any existing gapless metadata in the Info tag. The code in libmp3lame.c to set AV_PKT_DATA_SKIP_SAMPLES was mostly copied from libopusenc.c. Jon Toohill (3): lavc/libmp3lame: send encoder padding in side data of final packet lavf/mp3enc: write encoder delay/padding upon closing lavf/mp3dec: read encoder delay/padding from Info tag libavcodec/libmp3lame.c | 21 - libavformat/mp3dec.c | 3 ++- libavformat/mp3enc.c | 32 +--- tests/ref/fate/audiomatch-square-mp3 | 2 +- tests/ref/fate/gapless-mp3 | 10 +- 5 files changed, 53 insertions(+), 15 deletions(-) -- 2.8.0.rc3.226.g39d4020 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] lavf/mp3dec: read encoder delay/padding from Info tag
Muxers can check AVCodecParameters.initial_padding for the encoder+decoder delay, and read the AV_PKT_DATA_SKIP_SAMPLES side data from the last packet for the encoder padding. This change also fixes the first_discard_sample calculation which erroneously included the decoder delay. Decoder delay is already accounted for in st->skip_samples. The affected FATE tests have been updated accordingly. --- libavformat/mp3dec.c | 3 ++- tests/ref/fate/audiomatch-square-mp3 | 2 +- tests/ref/fate/gapless-mp3 | 10 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c index 56c7f8c..e8b2428 100644 --- a/libavformat/mp3dec.c +++ b/libavformat/mp3dec.c @@ -239,9 +239,10 @@ static void mp3_parse_info_tag(AVFormatContext *s, AVStream *st, mp3->start_pad = v>>12; mp3-> end_pad = v&4095; +st->codecpar->initial_padding = mp3->start_pad + 528 + 1; st->start_skip_samples = mp3->start_pad + 528 + 1; if (mp3->frames) { -st->first_discard_sample = -mp3->end_pad + 528 + 1 + mp3->frames * (int64_t)spf; +st->first_discard_sample = -mp3->end_pad + mp3->frames * (int64_t)spf; st->last_discard_sample = mp3->frames * (int64_t)spf; } if (!st->start_time) diff --git a/tests/ref/fate/audiomatch-square-mp3 b/tests/ref/fate/audiomatch-square-mp3 index 8de55c2..05176a0 100644 --- a/tests/ref/fate/audiomatch-square-mp3 +++ b/tests/ref/fate/audiomatch-square-mp3 @@ -1 +1 @@ -presig: 0 postsig:0 c: 0.9447 lenerr:0 +presig: 0 postsig:-529 c: 0.9334 lenerr:-529 diff --git a/tests/ref/fate/gapless-mp3 b/tests/ref/fate/gapless-mp3 index ebe7bfa..8b80bfc 100644 --- a/tests/ref/fate/gapless-mp3 +++ b/tests/ref/fate/gapless-mp3 @@ -1,5 +1,5 @@ -37534a3bcc3ef306e8c5ebfcfedfc41c *tests/data/fate/gapless-mp3.out-1 -c96c3ae7bd3300fd2f4debac222de5b7 -0cd1cdbcfd5cdbf6270cd98219bf31cd *tests/data/fate/gapless-mp3.out-2 -c96c3ae7bd3300fd2f4debac222de5b7 -9d3d8ba8a61b534f2d02ee648d6a8229 *tests/data/fate/gapless-mp3.out-3 +81695be427d45e8be4d527a6b2af2a85 *tests/data/fate/gapless-mp3.out-1 +c7879a827ab017364774069268d9a267 +62d074296f8c84a5f86a6afdd7bab459 *tests/data/fate/gapless-mp3.out-2 +c7879a827ab017364774069268d9a267 +e931f3fe1ba25e0d5eece4977c4061a9 *tests/data/fate/gapless-mp3.out-3 -- 2.8.0.rc3.226.g39d4020 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavformat/utils: ignore outlier subtitle and data stream end times as well
--- libavformat/utils.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/libavformat/utils.c b/libavformat/utils.c index 3acb260..ce68408 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -2524,7 +2524,7 @@ static int has_duration(AVFormatContext *ic) */ static void update_stream_timings(AVFormatContext *ic) { -int64_t start_time, start_time1, start_time_text, end_time, end_time1; +int64_t start_time, start_time1, start_time_text, end_time, end_time1, end_time_text; int64_t duration, duration1, filesize; int i; AVStream *st; @@ -2533,6 +2533,7 @@ static void update_stream_timings(AVFormatContext *ic) start_time = INT64_MAX; start_time_text = INT64_MAX; end_time = INT64_MIN; +end_time_text = INT64_MIN; duration = INT64_MIN; for (i = 0; i < ic->nb_streams; i++) { st = ic->streams[i]; @@ -2549,7 +2550,10 @@ static void update_stream_timings(AVFormatContext *ic) AV_ROUND_NEAR_INF|AV_ROUND_PASS_MINMAX); if (end_time1 != AV_NOPTS_VALUE && (end_time1 > 0 ? start_time1 <= INT64_MAX - end_time1 : start_time1 >= INT64_MIN - end_time1)) { end_time1 += start_time1; -end_time = FFMAX(end_time, end_time1); +if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE || st->codecpar->codec_type == AVMEDIA_TYPE_DATA) +end_time_text = FFMAX(end_time_text, end_time1); +else +end_time = FFMAX(end_time, end_time1); } for (p = NULL; (p = av_find_program_from_stream(ic, p, i)); ) { if (p->start_time == AV_NOPTS_VALUE || p->start_time > start_time1) @@ -2569,10 +2573,15 @@ static void update_stream_timings(AVFormatContext *ic) else if (start_time > start_time_text) av_log(ic, AV_LOG_VERBOSE, "Ignoring outlier non primary stream starttime %f\n", start_time_text / (float)AV_TIME_BASE); +if (end_time == INT64_MIN || (end_time < end_time_text && end_time_text - end_time < AV_TIME_BASE)) +end_time = end_time_text; +else if (end_time < end_time_text) +av_log(ic, AV_LOG_VERBOSE, "Ignoring outlier non primary stream endtime %f\n", end_time_text / (float)AV_TIME_BASE); + if (start_time != INT64_MAX) { ic->start_time = start_time; if (end_time != INT64_MIN) { -if (ic->nb_programs) { +if (ic->nb_programs > 1) { for (i = 0; i < ic->nb_programs; i++) { p = ic->programs[i]; if (p->start_time != AV_NOPTS_VALUE && p->end_time > p->start_time) -- 2.10.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check
On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote: > Decoders have previously not used AVFrame.pts, and with the upcoming > deprecation of pkt_pts (in favor of pts), this would lead to an errorneous > interpration of timestamps. I probably misunderstand the commit message but If code is changed in a user application that cannot really lift some blockage from changing a lib. a lib can only change in an incompaible way with (deprecation and) major version bump. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [GSoC] avcodec/als: Add ALS encoder
Hi, On Tue, Oct 4, 2016 at 1:53 AM, Thilo Borgmann wrote: > Hi, > >> Patch attached. >> >> It fixes the fate tests. >> However, there's a slight bug in the encoder in handling the last frame. >> I'll definitely fix it later. >> I hope the patch can be merged in this state. > > no. The last frame has to be handled properly before merging happens. > I'm at LinuxCon & ELCE for the next two weeks, so don't expect too much input > from me before Oct. 14th. Please fix whatever is possible until then. > > -Thilo Okay!! I'm on it. I should be able to fix it. - Umair >> On Mon, Sep 12, 2016 at 12:53 PM, Umair Khan wrote: >> >>> Hi, >>> >>> Sorry for late reply. I was travelling a bit. >>> Attached is the patch which includes the following changes than the >>> previous one - >>> >>> 1. Removed changes of the file libavformat/movenc.c, as I had added >>> this file in the initial commit by mistake. >>> 2. Removed the assembly code. >>> 3. Make changes as suggested by Michael (av_assertX and header_size). >>> >>> As far as fate tests are concerned, I haven't checked that yet. I'll >>> see what's the issue there. >>> >>> On Mon, Aug 29, 2016 at 11:34 PM, Michael Niedermayer >>> wrote: On Mon, Aug 29, 2016 at 10:47:59PM +0530, Umair Khan wrote: > Hi, > > On Sun, Aug 28, 2016 at 4:26 PM, Michael Niedermayer > wrote: >> On Sun, Aug 28, 2016 at 01:34:46PM +0530, Umair Khan wrote: >>> Hi, >>> >>> Patches attached. :) >>> >>> - Umair >> >>> Changelog |1 + >>> 1 file changed, 1 insertion(+) >>> d3f30e62d803d967bd5c27dc5dfad278ce5c02e9 >>> 0001-Changelog-Add-entry-for-ALS-encoder.patch >>> From 020370545a82c8c1304ec9c177a75e27e59b84e8 Mon Sep 17 00:00:00 >>> 2001 >>> From: Umair Khan >>> Date: Sat, 27 Aug 2016 22:22:02 +0530 >>> Subject: [PATCH 1/2] Changelog: Add entry for ALS encoder >>> >>> Signed-off-by: Umair Khan >>> --- >>> Changelog | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/Changelog b/Changelog >>> index b903e31..90c15ad 100644 >>> --- a/Changelog >>> +++ b/Changelog >>> @@ -15,6 +15,7 @@ version : >>> - True Audio (TTA) muxer >>> - crystalizer audio filter >>> - acrusher audio filter >>> +- ALS encoder >>> >>> >>> version 3.1: >>> -- >>> 2.7.4 (Apple Git-66) >>> >> >> [...] >>> +static int calc_short_term_prediction(ALSEncContext *ctx, ALSBlock >>> *block, >>> + int order) >>> +{ >>> +ALSSpecificConfig *sconf = &ctx->sconf; >>> +int i, j; >>> + >>> +int32_t *res_ptr = block->res_ptr; >>> +int32_t *smp_ptr = block->cur_ptr; >>> + >>> +assert(order > 0); >> >> should be av_assertX (X=0/1/2) >> >> >> [...] >>> +int ff_window_init(WindowContext *wctx, enum WindowType type, int >>> length, >>> + double param) >>> +{ >>> +if (!length || length < -1) >>> +return AVERROR(EINVAL); >>> + >>> +wctx->type = type; >>> +wctx->length = length; >>> +wctx->param = param; >>> + >>> +switch (type) { >>> +case WINDOW_TYPE_RECTANGLE: >>> +rectangle_init(wctx); >>> +break; >>> +case WINDOW_TYPE_WELCH: >>> +WINDOW_INIT(welch) >>> +break; >>> +case WINDOW_TYPE_SINERECT: >>> +WINDOW_INIT(sinerect) >>> +break; >>> +case WINDOW_TYPE_HANNRECT: >>> +WINDOW_INIT(hannrect) >>> +break; >>> +default: >>> +return AVERROR(EINVAL); >>> +} >>> + >> >>> +if (HAVE_MMX) >>> +ff_window_init_mmx(wctx); >> >> breaks build on non x86 as the function declaration / prototype is >> not there in that case > > What should I do with this then? I'm not too aware of how the whole > code works because I didn't originally write it. > So, I'll need some help here. :) IIRC the declaration / prototype is under #if but the call is not under #if thus if the condition on the #if is untrue this fails to build (this is not the same as the functions implementation being missing for if(0) code, that one works with all supported platforms) its probably best to split the whole *_mmx code out into a seperate patch and also either the call must be under #if or the declaration must be available independant of an #if [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Democracy is the form of government in which you can choose your dictator ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >>> >>> >>> _
[FFmpeg-devel] [PATCH 4/4] ffprobe: report field order for video streams
--- ffprobe.c | 13 + tests/ref/fate/concat-demuxer-extended-lavf-mxf | 2 +- tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 | 2 +- tests/ref/fate/concat-demuxer-simple1-lavf-mxf | 2 +- tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 | 2 +- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 2 +- tests/ref/fate/ffprobe_compact | 4 ++-- tests/ref/fate/ffprobe_csv | 4 ++-- tests/ref/fate/ffprobe_default | 2 ++ tests/ref/fate/ffprobe_flat | 2 ++ tests/ref/fate/ffprobe_ini | 2 ++ 11 files changed, 28 insertions(+), 9 deletions(-) diff --git a/ffprobe.c b/ffprobe.c index bb3979c..3118e80 100644 --- a/ffprobe.c +++ b/ffprobe.c @@ -2268,6 +2268,19 @@ static int show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_id else print_str_opt("chroma_location", av_chroma_location_name(par->chroma_location)); +if (par->field_order == AV_FIELD_PROGRESSIVE) +print_str("field_order", "progressive"); +else if (par->field_order == AV_FIELD_TT) +print_str("field_order", "tt"); +else if (par->field_order == AV_FIELD_BB) +print_str("field_order", "bb"); +else if (par->field_order == AV_FIELD_TB) +print_str("field_order", "tb"); +else if (par->field_order == AV_FIELD_BT) +print_str("field_order", "bt"); +else +print_str_opt("field_order", "unknown"); + #if FF_API_PRIVATE_OPT if (dec_ctx && dec_ctx->timecode_frame_start >= 0) { char tcbuf[AV_TIMECODE_STR_SIZE]; diff --git a/tests/ref/fate/concat-demuxer-extended-lavf-mxf b/tests/ref/fate/concat-demuxer-extended-lavf-mxf index f7905aa..8bb2fb0 100644 --- a/tests/ref/fate/concat-demuxer-extended-lavf-mxf +++ b/tests/ref/fate/concat-demuxer-extended-lavf-mxf @@ -1 +1 @@ -21eb3a629ff504b55c93a66879a31362 *tests/data/fate/concat-demuxer-extended-lavf-mxf.ffprobe +a277e04c23cf764abe692ca07e87b82e *tests/data/fate/concat-demuxer-extended-lavf-mxf.ffprobe diff --git a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 index 0c49f1f..e294538 100644 --- a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 +++ b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 @@ -1 +1 @@ -67a03ad49f1bd17131f751313639b61e *tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe +026045a43aa2dde1723d7331c2252b01 *tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe diff --git a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf index 6bba76a..c899754 100644 --- a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf +++ b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf @@ -120,5 +120,5 @@ audio|1|65280|1.36|65280|1.36|1920|0.04|N/A|N/A|3840|206848|K_|1 Strings Metadata|8 video|0|37|1.48|34|1.36|1|0.04|N/A|N/A|24786|211456|K_|1 Strings Metadata|8 -0|mpeg2video|4|video|1/25|[0][0][0][0]|0x|352|288|0|0|1|1:1|11:9|yuv420p|8|tv|unknown|unknown|unknown|left|N/A|1|N/A|25/1|25/1|1/25|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|51|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301 +0|mpeg2video|4|video|1/25|[0][0][0][0]|0x|352|288|0|0|1|1:1|11:9|yuv420p|8|tv|unknown|unknown|unknown|left|progressive|N/A|1|N/A|25/1|25/1|1/25|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|51|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301 1|pcm_s16le|unknown|audio|1/48000|[0][0][0][0]|0x|s16|48000|1|unknown|16|N/A|0/0|0/0|1/48000|0|0.00|N/A|N/A|768000|N/A|N/A|N/A|N/A|50|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301 diff --git a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 index 75cac84..2ba3a2e 100644 --- a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 +++ b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 @@ -78,5 +78,5 @@ video|0|34|1.36|34|1.36|1|0.04|N/A|N/A|15|1923072|K_|1 Strings Metadata|8 audio|1|65280|1.36|65280|1.36|1920|0.04|N/A|N/A|7680|2073600|K_|1 Strings Metadata|8 -0|mpeg2video|0|video|1/25|[0][0][0][0]|0x|720|608|0|0|0|1:1|45:38|yuv422p|5|tv|unknown|unknown|unknown|topleft|N/A|1|N/A|25/1|25/1|1/25|0|0.00|N/A|N/A|3000|N/A|N/A|N/A|N/A|35|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301 +0|mpeg2video|0|video|1/25|[0][0][0][0]|0x|720|608|0|0|0|1:1|45:38|yuv422p|5|tv|unknown|unknown|unknown|topleft|tt|N/A|1|N/A|25/1|25/1|1/25|0|0.00|N/A|N/A|3000|N/A|N/A|N/A|N/A|35|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301 1|pcm_s16le|unknown|audio|1/48000|[0][0][0][0]|0x|s16|48000|2|un
[FFmpeg-devel] [PATCH 3/4] lavc/utils: avcodec_string: dump field order when known
--- libavcodec/utils.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 6f4df93..a0931c6 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -3228,6 +3228,21 @@ void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode) av_get_colorspace_name(enc->colorspace)); } +if (enc->field_order != AV_FIELD_UNKNOWN) { +const char *field_order = "progressive"; +if (enc->field_order == AV_FIELD_TT) +field_order = "top first"; +else if (enc->field_order == AV_FIELD_BB) +field_order = "bottom first"; +else if (enc->field_order == AV_FIELD_TB) +field_order = "top coded first, swapped"; +else if (enc->field_order == AV_FIELD_BT) +field_order = "bottom coded first, swapped"; + +av_strlcatf(detail, sizeof(detail), "%s, ", field_order); +} + + if (av_log_get_level() >= AV_LOG_DEBUG && enc->chroma_sample_location != AVCHROMA_LOC_UNSPECIFIED) av_strlcatf(detail, sizeof(detail), "%s, ", -- 2.10.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/4] lavc/h264_parser: export field order in more cases
--- libavcodec/h264_parser.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c index 3ed7d77..bca0071 100644 --- a/libavcodec/h264_parser.c +++ b/libavcodec/h264_parser.c @@ -61,6 +61,7 @@ typedef struct H264ParseContext { int parse_history_count; int parse_last_mb; int64_t reference_dts; +int last_frame_num, last_picture_structure; } H264ParseContext; @@ -528,7 +529,19 @@ static inline int parse_nal_units(AVCodecParserContext *s, s->picture_structure = AV_PICTURE_STRUCTURE_TOP_FIELD; else s->picture_structure = AV_PICTURE_STRUCTURE_BOTTOM_FIELD; -s->field_order = AV_FIELD_UNKNOWN; +if (p->poc.frame_num == p->last_frame_num && +p->last_picture_structure != AV_PICTURE_STRUCTURE_UNKNOWN && +p->last_picture_structure != AV_PICTURE_STRUCTURE_FRAME && +p->last_picture_structure != s->picture_structure) { +if (p->last_picture_structure == AV_PICTURE_STRUCTURE_TOP_FIELD) +s->field_order = AV_FIELD_TT; +else +s->field_order = AV_FIELD_BB; +} else { +s->field_order = AV_FIELD_UNKNOWN; +} +p->last_picture_structure = s->picture_structure; +p->last_frame_num = p->poc.frame_num; } av_freep(&nal.rbsp_buffer); @@ -677,6 +690,7 @@ static av_cold int init(AVCodecParserContext *s) H264ParseContext *p = s->priv_data; p->reference_dts = AV_NOPTS_VALUE; +p->last_frame_num = INT_MAX; ff_h264dsp_init(&p->h264dsp, 8, 1); return 0; } -- 2.10.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/4] lavc/parser: export field order if not already set
Some codecs set this in the parser, but not the decoder --- libavcodec/parser.c | 5 + 1 file changed, 5 insertions(+) diff --git a/libavcodec/parser.c b/libavcodec/parser.c index 2c8fc69..30cfc55 100644 --- a/libavcodec/parser.c +++ b/libavcodec/parser.c @@ -182,6 +182,11 @@ int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx, index = s->parser->parser_parse(s, avctx, (const uint8_t **) poutbuf, poutbuf_size, buf, buf_size); av_assert0(index > -0x2000); // The API does not allow returning AVERROR codes +#define FILL(name) if(s->name > 0 && avctx->name <= 0) avctx->name = s->name +if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) { +FILL(field_order); +} + /* update the file pointer */ if (*poutbuf_size) { /* fill the data for the current frame */ -- 2.10.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc: set best effort timestamp if unset when using new decode API
On 10/2/2016 6:56 PM, wm4 wrote: > Some API users (in particular ffmpeg.c) check the best effort timestamp > only. > --- > Still undecided if this is the right approach. > --- > libavcodec/utils.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index ef3da65..1875a69 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -2867,7 +2867,12 @@ int attribute_align_arg > avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr > if (avctx->codec->receive_frame) { > if (avctx->internal->draining && !(avctx->codec->capabilities & > AV_CODEC_CAP_DELAY)) > return AVERROR_EOF; > -return avctx->codec->receive_frame(avctx, frame); > +ret = avctx->codec->receive_frame(avctx, frame); > +if (ret >= 0) { > +if (av_frame_get_best_effort_timestamp(frame) == AV_NOPTS_VALUE) > +av_frame_set_best_effort_timestamp(frame, frame->pkt_pts); > +} > +return ret; > } > > // Emulation via old API. > +1 from me. Not a single decoder sets this, and requiring it for the new API seems a bit strange. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Developer needed for applying watermark onto video on server
On 10/3/2016 11:09 PM, chuck wrote: > Hi, I am looking for a developer for hire to help us with an issue > applying a watermark to videos after they are uploaded and converted on > the server. I am the project manager and do not have much technical > information but you will be able to speak with our coders directly. Our > deadline is roughly 10 days. Please respond with an approximate cost for > this project. Just in general, the watermark should be applied during the conversion. If you are already using ffmpeg for that, the entire watermark thing is just one video filter overlaying an image, a minor addition to the commandline. Or do you mean a hidden watermark to identify the source of the video? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 4/4] ffprobe: report field order for video streams
On Mon, Oct 03, 2016 at 11:49:39PM -0500, Rodger Combs wrote: > --- > ffprobe.c | 13 + > tests/ref/fate/concat-demuxer-extended-lavf-mxf | 2 +- > tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 | 2 +- > tests/ref/fate/concat-demuxer-simple1-lavf-mxf | 2 +- > tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 | 2 +- > tests/ref/fate/concat-demuxer-simple2-lavf-ts | 2 +- > tests/ref/fate/ffprobe_compact | 4 ++-- > tests/ref/fate/ffprobe_csv | 4 ++-- > tests/ref/fate/ffprobe_default | 2 ++ > tests/ref/fate/ffprobe_flat | 2 ++ > tests/ref/fate/ffprobe_ini | 2 ++ > 11 files changed, 28 insertions(+), 9 deletions(-) > > diff --git a/ffprobe.c b/ffprobe.c > index bb3979c..3118e80 100644 > --- a/ffprobe.c > +++ b/ffprobe.c > @@ -2268,6 +2268,19 @@ static int show_stream(WriterContext *w, > AVFormatContext *fmt_ctx, int stream_id > else > print_str_opt("chroma_location", > av_chroma_location_name(par->chroma_location)); > > +if (par->field_order == AV_FIELD_PROGRESSIVE) > +print_str("field_order", "progressive"); > +else if (par->field_order == AV_FIELD_TT) > +print_str("field_order", "tt"); > +else if (par->field_order == AV_FIELD_BB) > +print_str("field_order", "bb"); > +else if (par->field_order == AV_FIELD_TB) > +print_str("field_order", "tb"); > +else if (par->field_order == AV_FIELD_BT) > +print_str("field_order", "bt"); > +else > +print_str_opt("field_order", "unknown"); > + This probably needs an update of doc/ffprobe.xsd [...] -- Clément B. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] DXVA2: Fix crash releasing IDirect3D9Surface's with av_buffer_default_free instead of Release
Hi, I'm using ffmpeg dxva2 implementation (ffmpeg_dxva2.c) to render mp4 video to a DX texture. The crash happens when closing my application, when I call to av_frame_free() to the last frame I've created to decompress video. The hwcontext references go to 0 and internal structs start to free its memory, but with DXVA2FramesContext::surfaces_internal I've noticed that is an array with IDirect3D9Surfaces's. You can see it in the file libavutil/hwcontext_dxva2.c:169, inside function dxva2_init_pool(). hr = IDirectXVideoAccelerationService_CreateSurface(s->service, ctx->width, ctx->height, ctx->initial_pool_size - 1, s->format, D3DPOOL_DEFAULT, 0, frames_hwctx->surface_type, s->surfaces_internal, NULL); In the same file, the function dxva2_pool_alloc() calls av_buffer_create() with every surface created before, but the "free" function pointer parameter is passed as NULL, and in that case av_buffer_default_free() will be called for every surface, and that's not correct. I've created the patch to avoid this case, and I've checked that there is no memory leaks using debug CRT with visual studio. The surfaces are properly released in the function dxva2_frames_uninit(). I don't know exactly how to reproduce using ffmpeg command line because what I'm doing is integrating ffmeg in my 3d engine, but I supose that if you decompress using dxva2 it will crash when finishing ffmpeg app, if all the reference counting is ok and all the context and frames are freed. Sorry if I'm wrong, but what I've seen with the visual studio debugger and what I can see in the code makes sense for me . Greetings Min 2016-09-30 8:23 GMT+02:00 Hendrik Leppkes : > On Fri, Sep 30, 2016 at 7:48 AM, Min wrote: > > diff --git a/libavutil/hwcontext_dxva2.c b/libavutil/hwcontext_dxva2.c > > index e79254b..17d8eb5 100644 > > --- a/libavutil/hwcontext_dxva2.c > > +++ b/libavutil/hwcontext_dxva2.c > > @@ -101,6 +101,11 @@ static void dxva2_frames_uninit(AVHWFramesContext > *ctx) > > } > > } > > > > +void dxva2_pool_free(void *opaque, uint8_t *data) > > +{ > > +// No need to free surfaces here, they will be Released later > > +} > > + > > static AVBufferRef *dxva2_pool_alloc(void *opaque, int size) > > { > > AVHWFramesContext *ctx = (AVHWFramesContext*)opaque; > > @@ -110,7 +115,7 @@ static AVBufferRef *dxva2_pool_alloc(void *opaque, > int > > size) > > if (s->nb_surfaces_used < hwctx->nb_surfaces) { > > s->nb_surfaces_used++; > > return > > av_buffer_create((uint8_t*)s->surfaces_internal[s->nb_surfaces_used - > 1], > > -sizeof(*hwctx->surfaces), NULL, 0, 0); > > +sizeof(*hwctx->surfaces), > dxva2_pool_free, > > 0, 0); > > } > > > > return NULL; > > This is not correct and will leak memory. The buffer created here is > not a "surface", its plain memory to hold information about a surface, > and needs to be free'ed properly when its no longer used. > On top of all that, DXVA2 usage through ffmpeg.c, for example, also > doesn't crash here. > > So, how exactly does one reproduce the problem you're trying to fix? > > - Hendrik > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check
On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer wrote: > On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote: >> Decoders have previously not used AVFrame.pts, and with the upcoming >> deprecation of pkt_pts (in favor of pts), this would lead to an errorneous >> interpration of timestamps. > > I probably misunderstand the commit message but > If code is changed in a user application that cannot really lift > some blockage from changing a lib. > a lib can only change in an incompaible way with (deprecation and) > major version bump. > The lib never did what this code suggests it did, not that I remember (so at least not for a long long time). - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel