[Ffmpeg-devel-irc] ffmpeg-devel.log.20171102

[burek]

[00:00:45 CET] <Compn> mp4als ?
[00:00:54 CET] Action: Compn looks at github and github stares backj
[00:00:59 CET] <durandal_1707> lol, no
[00:03:57 CET] <atomnuker> durandal_1707: spcr3?
[00:04:05 CET] <atomnuker> *scpr3
[00:04:18 CET] <durandal_1707> yes
[00:07:05 CET] <atomnuker> oh wow, that's a huge patch
[00:07:31 CET] <durandal_1707> its wip
[00:07:38 CET] <cone-975> ffmpeg 03Diego Biurrun 07master:97cfe1d8bd19: Convert all AVClass struct declarations to designated initializers.
[00:07:39 CET] <cone-975> ffmpeg 03James Almer 07master:e621b1ca646a: Merge commit '97cfe1d8bd1968143e2ba9aa46ebe9504a835e24'
[00:07:46 CET] <durandal_1707> need lot of refactoring
[00:08:27 CET] <Compn> neat
[00:08:28 CET] <Compn> https://twitter.com/kieranjol/status/921341912921067521
[00:08:37 CET] <Compn> irish film institute uses ffv1/mkv :)
[00:08:50 CET] <Compn> michaelni : your codec is going to live forever! haha
[00:10:04 CET] <jamrial> Compn: makes sense, seeing how the standarization efforts for mkv and ffv1 are apparently handled together
[00:11:26 CET] <Compn> ah
[00:11:27 CET] <jamrial> matroska + ffv1/flac is trying to be introduced as an alternative to mxf + jpeg2k (which seems to be popular in some environments), or so i've been told
[00:12:05 CET] <Compn> yeah i remember jp2k was supposed to be "it" and then i saw a lot of people switch to ffv1
[00:12:14 CET] <Compn> people/orgs
[00:15:47 CET] <llogan> the archive people are quite active on twitter. see many ffv1 tweets from them.
[00:18:29 CET] <cone-975> ffmpeg 03Diego Biurrun 07master:d76479c5020c: ppc: Drop support for Apple GCC
[00:18:30 CET] <cone-975> ffmpeg 03James Almer 07master:2eb20caccf0f: Merge commit 'd76479c5020ca43e67d47ba3767146b192dc4782'
[00:20:01 CET] <cone-975> ffmpeg 03Diego Biurrun 07master:e2edf1529cb3: configure: Simplify AltiVec/VSX check with a helper function
[00:20:02 CET] <cone-975> ffmpeg 03James Almer 07master:5352db1dd0f8: Merge commit 'e2edf1529cb35eaf043e3f8e5cba498ed06e2563'
[00:22:14 CET] <Compn> https://twitter.com/VideoTechStuff/status/919863054065692674
[00:22:16 CET] <Compn> "need samples"
[00:22:22 CET] <cone-975> ffmpeg 03Diego Biurrun 07master:04f691cd4fb7: mmal: Add missing .item_name to AVClass declaration
[00:22:23 CET] <cone-975> ffmpeg 03James Almer 07master:f3f1bbc6a88b: Merge commit '04f691cd4fb7d226e54df886a21ba201cb5019f4'
[00:24:29 CET] <nevcairiel> its a network protocol, samples not so easy =p
[00:26:04 CET] <cone-975> ffmpeg 03Michael Niedermayer 07master:f61265571d68: libfdk-aacdec: Correct buffer_size parameter
[00:26:05 CET] <cone-975> ffmpeg 03Mark Thompson 07master:4993a68f0f92: hwcontext: Mark local table static const
[00:26:06 CET] <cone-975> ffmpeg 03James Almer 07master:862c85b2ff0d: Merge commit '4993a68f0f9285f92a42a54305dc0244665b7db4'
[00:35:39 CET] <cone-975> ffmpeg 03Diego Biurrun 07master:de099641d556: build: Add missing mpeg4audio dependency for RTP muxer
[00:35:40 CET] <cone-975> ffmpeg 03Diego Biurrun 07master:2f792cb6703b: build: Add missing idctdsp dependency for clearvideo
[00:35:41 CET] <cone-975> ffmpeg 03Diego Biurrun 07master:9d12dd6fa84d: configure: Add x86 dependency for mmx_internal
[00:35:42 CET] <cone-975> ffmpeg 03James Almer 07master:580732e3a5d7: Merge commit '2f792cb6703b5b12f2e873bee13f33da8aa9940a'
[00:35:43 CET] <cone-975> ffmpeg 03James Almer 07master:7116e6894b47: Merge commit '9d12dd6fa84d9f6094bdf0fdae62527ecec0bbb6'
[00:41:16 CET] <cone-975> ffmpeg 03Diego Biurrun 07master:c599c43fa147: configure: Add missing arch dependencies for arch extensions
[00:41:17 CET] <cone-975> ffmpeg 03James Almer 07master:6c35f8906783: Merge commit 'c599c43fa147d7a001cdbf8f3b7da0a965f2e285'
[00:53:05 CET] <cone-975> ffmpeg 03Diego Biurrun 07master:635897ac78ef: configure: Factorize qsv dependencies
[00:53:06 CET] <cone-975> ffmpeg 03James Almer 07master:9f10052c9d3e: Merge commit '635897ac78ef29869f0321ab921c91b3e1aad453'
[00:57:11 CET] <cone-975> ffmpeg 03Diego Biurrun 07master:7b1f9873be2f: build: Adjust dependencies for faan(i)dct components
[00:57:12 CET] <cone-975> ffmpeg 03James Almer 07master:7e5ebd0009a2: Merge commit '7b1f9873be2f9d5aea2762c9197ff46df82768dc'
[01:00:14 CET] <cone-975> ffmpeg 03Diego Biurrun 07master:d9da7151eef7: configure: Fix handling of _select dependencies
[01:00:15 CET] <cone-975> ffmpeg 03Mark Thompson 07master:5635c80bf59d: vf_hwmap: Add missing error code
[01:00:16 CET] <cone-975> ffmpeg 03Mark Thompson 07master:a670eea56087: vf_hwmap: Properly free a locally derived device
[01:00:17 CET] <cone-975> ffmpeg 03Mark Thompson 07master:c2b0bea40f1f: avconv_hw: Free device on initialisation failure
[01:00:18 CET] <cone-975> ffmpeg 03James Almer 07master:1076f93a94cc: Merge commit 'c2b0bea40f1fd4399ff6184a2df4f397c0f4b3ab'
[01:02:54 CET] <cone-975> ffmpeg 03Martin Storsjö 07master:fd1ffa1f10e9: d3d11va: Link directly to dxgi.dll and d3d11.dll functions if LoadLibrary is unavailable
[01:02:55 CET] <cone-975> ffmpeg 03James Almer 07master:5ab41a25ba9c: Merge commit 'fd1ffa1f10e940165035ccb79d4a6523da196062'
[04:20:51 CET] <marsrover> is there a way to enable only the audio filters/disable all the video filters?
[04:21:35 CET] <marsrover> i mean without manually going through the whole list doing --disable-<whatever>
[12:24:17 CET] <cone-181> ffmpeg 03Nicolas George 07master:345e7072ab86: lavfi: check links properties after configuring them.
[12:24:18 CET] <cone-181> ffmpeg 03Nicolas George 07master:d5995c531d6e: lavfi/framepool: use av_image_check_size2().
[15:17:20 CET] <kierank> michaelni: I am going to send a patch reverting h264 dec NAL code to the old code
[15:17:23 CET] <kierank> are you ok with this?
[15:18:16 CET] <JEEB> I'm not sure if the separate buffer thing can be easily fixed, so that sounds better than OOMs
[15:23:17 CET] <durandal_1707> kierank: why it was changed at first at all?
[15:24:01 CET] <kierank> durandal_1707: libav cleaning of course
[15:25:00 CET] <kierank> https://github.com/FFmpeg/FFmpeg/commit/772ad7142dff590c7bb30370acf5d3c9a8fb512b
[15:25:28 CET] <JEEB> man that's old by now :D
[15:26:57 CET] <michaelni> kierank, i need to look (will do once teres a patch) but as long as it doesnt interfere with future features or slows anyting real world down, sure whatever works better
[15:27:16 CET] <kierank> michaelni: well the merge made things slower but nobody cared
[15:27:22 CET] <kierank> so it's a stupid assessment
[15:27:38 CET] <kierank> the merge code is unacceptable in a 24/7 environment as derek explained
[15:27:53 CET] <kierank> michaelni: all I'm going to do is the opposite of https://github.com/FFmpeg/FFmpeg/commit/772ad7142dff590c7bb30370acf5d3c9a8fb512b
[15:27:59 CET] <kierank> which i can't revert because of filename changes
[15:31:09 CET] <cone-181> ffmpeg 03Steven Liu 07master:c52beb4839e0: avformat/hlsenc: check hls segment mode for ignore the init filename
[15:31:10 CET] <cone-181> ffmpeg 03Steven Liu 07master:c7360512c27d: avformat/hlsenc: reindent hlsenc code
[15:34:05 CET] <kierank> michaelni: do you have any comments on that approach?
[15:36:48 CET] <jamrial> kierank: i'd very much like that whatever gets applied is applied in both projects
[15:36:55 CET] <jamrial> since the problem in shared
[15:37:09 CET] <jamrial> reverting that amount of code would make future merges of h264 virtually impossible
[15:37:24 CET] <kierank> well to be frank I'm not convincing libav to do that
[15:37:52 CET] <kierank> I can't spend 3 days debugging issues because of their inate desire for cleanliness
[15:38:16 CET] <kierank> and then have to fight them to get these problems resolved
[15:44:39 CET] <lrusak> jkqxz: do you know the answer here? https://github.com/mpv-player/mpv/commit/05cb8d28afd8881667c3dea297b182a2ecdd91c8#commitcomment-25317019
[16:48:27 CET] <nevcairiel> kierank: cant you just change how the memory is allocated or whatever, instead of reverting some huge commit that touches basically the entire decoder?
[16:48:48 CET] <kierank> nevcairiel: no, the point is that the memory shouldn't be allocated in the first place
[16:49:07 CET] <nevcairiel> if you do unescaping, you need a buffer anyway
[16:49:22 CET] <kierank> the old code didn't allocate
[16:49:36 CET] <kierank> not sure the code is unescaping
[16:49:40 CET] <kierank> just copying nal data around
[16:52:08 CET] <nevcairiel> the only allocation of any noticeable size is the buffer to unescape into
[16:52:28 CET] <nevcairiel> which the nal parsing does
[16:54:03 CET] <nevcairiel> the old code did that as well, but it stored it differently
[16:56:06 CET] <nevcairiel> the only bad part seems to be that these H2645Packet structs are being reused without limit so their allocations can potentially grow
[16:57:05 CET] <nevcairiel> perhaps there should be soem cleanup somewhere to keep them in check
[17:01:07 CET] <kierank> these allocations are per thread
[17:01:14 CET] <kierank> and malloc+free is "too slow"
[17:01:25 CET] <kierank> so we lock ourselves into multiple catch 22s here
[17:13:11 CET] <jamrial> kierank: i don't think you'd have to fight if you report an issue with some code elenril wrote
[17:13:38 CET] <kierank> 9:04:27 PM <"kierank> elenril: regarding h2645_parse, are you aware you can cause an OOM if you have many NALs in your packet of certain sizes?
[17:13:38 CET] <kierank> 9:04:52 PM <"kierank> Because the nal cache can be blown
[17:13:38 CET] <kierank> 9:10:58 PM <"wm4> huh why would it alloc something per NAL (and keep the allocation)
[17:13:38 CET] <kierank> 9:19:06 PM <"kierank> That's what it does
[17:13:38 CET] <kierank> 10:57:33 PM <"elenril> kierank: on a real file?
[17:13:45 CET] <kierank> I have machines OOMing every day
[17:13:50 CET] <kierank> because of this crappy code
[17:13:54 CET] <jamrial> i reported an issue with the frame cropping code he wrote, he looked at it and fixed it
[17:14:32 CET] <nevcairiel> i dont know what your quote is supposed to proof, but he just asked if its a real file, and if you can give him one he will likely take it seriously
[17:14:40 CET] <jamrial> if you can give him a way to reproduce this he will look at it
[17:15:26 CET] <kierank> it's not an issue that occurs with files
[17:15:33 CET] <kierank> it occurs with long running video sources
[17:15:42 CET] <nevcairiel> any broadcast stream is also a file once its recorded
[17:16:26 CET] <jamrial> can't you set up a streaming source he can use?
[17:16:36 CET] <kierank> https://trac.ffmpeg.org/ticket/6789#comment:2 has a way of reproduing
[17:16:51 CET] <kierank> I can't see how this bug can be fixed without awful hacks
[17:16:54 CET] <jamrial> i mean, this is a big issue in both projects. reverting the offending code in one is not exactly ideal
[17:17:18 CET] <kierank> they shouldn't go on big refactoring hacks and these hacks shouldn't just get merged
[17:17:21 CET] <kierank> h264 is very complex
[17:17:36 CET] <nevcairiel> reverting such a huge commit also screams awful hack to me, or even worse "I don't understand the issue, but this code worked"
[17:18:05 CET] <kierank> nonsense, the existing code did the nal segmentation in place, the new one does it in a temporary buffer that grows infinitely
[17:18:07 CET] <kierank> per thread
[17:19:32 CET] <kierank> every time i upgrade ffmpeg I always get merge related bugs
[17:20:32 CET] <nevcairiel> if there is no escapes its still done in place, and if there is escapes, its still done in a temporary buffer - just like the old code
[17:21:24 CET] <jamrial> yeah, and then they get fixed, and the fix submitted back to source or merged from it
[17:21:47 CET] <jamrial> what i don't get is how it took more than a year for this to issue to become obvious
[17:21:53 CET] <nevcairiel> how bad can the performance hit be to free that packet buffer once per frame?
[17:21:54 CET] <kierank> because it's a complex and subtle bug
[17:21:57 CET] <kierank> nevcairiel: dunno, ask michael
[17:22:13 CET] <kierank> and difficult to reproduce consistently
[17:22:57 CET] <kierank> First time I saw it, the machine OOMd after 43 days
[17:24:09 CET] <nevcairiel> the only numbers even remotely related to that thing where in that patch on the  ML, which used an entirely different approach, noone ever mentioned just freeing those structs
[17:27:11 CET] <kierank> you still leak, just slower with that patch
[17:27:14 CET] <kierank> so it's pointless
[17:27:35 CET] <nevcairiel> I realize, however its still the only approach i ever saw someone post benchmarks of
[17:29:26 CET] <kierank> I don't understand what you want me to do, I can show you that trac ticket leaking again with the new code but it doesn't change anything
[17:29:29 CET] <kierank> the world is not a file
[17:29:36 CET] <kierank> where ffmpeg finishes and you see a max memory usage
[17:29:56 CET] <kierank> https://usercontent.irccloud-cdn.com/file/zvbVa9CC/image.png
[17:29:58 CET] <kierank> here is my pretty picture
[17:30:48 CET] <nevcairiel> i want you to consider the possibility of fixing the runaway allocations instead of just giving up and reverting =p
[17:30:55 CET] <kierank> there is no way
[17:31:03 CET] <kierank> that will satisfy the people who think malloc and free is too much
[17:33:04 CET] <nevcairiel> I don't see anyone claiming that quite yet
[17:33:24 CET] <kierank> the problem is michaelni wants a speed comparison between any new code and the existing code
[17:33:32 CET] <kierank> yet the existing code was merged without comparison with the old code
[17:33:35 CET] <kierank> so it's completely bogus
[17:34:00 CET] <nevcairiel> being aware of a speed difference doesn't necessarily mean it would rule out such a change entirely
[17:34:18 CET] <kierank> unless you allocate and free you will always end up with the worst-case NAL sizing in memory
[17:34:22 CET] <kierank> that's the fundamental problem
[17:34:38 CET] <kierank> and if nobody wants to do anything about it reverting is the best way
[17:39:10 CET] <nevcairiel> from what I can tell, the old code is equally prone to a similar issue, just perhaps not quite as pronounced, but it also has a persistent shared buffer, but maybe not quite as many of them
[17:39:34 CET] <nevcairiel> using one approach of moving the shared buffer out of the nal struct into the packet struct would bring it to similar behavior as the old code
[17:45:13 CET] <kierank> there is no equivalent H264SliceContext in the parser
[17:46:09 CET] <kierank> this is mpegvideo.c all over again
[17:46:44 CET] <nevcairiel> actually its the opposite, it moves from one centralized crazy struct to modular re-used components
[17:47:26 CET] <kierank> and causing O(n^2) allocs...
[17:47:34 CET] <kierank> via that abstraction
[17:49:03 CET] <nevcairiel> that doesnt necessarily have to be the case though, the decoder holds exactly one H2645Packet (per thread), so if you were to move the buffer into  there instead of having it per-nal, you would already reduce allocations quite a bit and resolves the runaway nal-count problem. still prone to run-away nal size though, but the old code was that as well.
[17:51:52 CET] <kierank> the code is shared with the parser
[17:51:59 CET] <kierank> and the parser doesnt have an obvious place to put this
[17:52:13 CET] <nevcairiel> all calling code has to have a H2645Packet
[17:53:29 CET] <nevcairiel> actually looks like the parser re-invents most of that anyway
[17:53:50 CET] <nevcairiel> it currently does allocations for every frame already, so nothing really lost there
[17:56:16 CET] <kierank> ff_h2645_extract_rbsp has no apcket
[17:56:17 CET] <kierank> packet
[17:56:57 CET] <nevcairiel> yeah that takes the nal, but the parser doesnt store that persistently between invocations of itself anyway, so whatever changes are required it could handle it
[18:01:37 CET] <kierank> so I can make it H2645RBSP or whatever
[18:02:09 CET] <kierank> ok i have a rough idea how to proeced
[18:02:10 CET] <kierank> thanks nevcairiel 
[18:02:49 CET] <kierank> I have a rough idea how to fix ffmpeg problems
[18:03:29 CET] <kierank> bah wrong channel
[18:03:29 CET] <nevcairiel> if you want to test the basic idea of malloc+free  for every frame, https://pastebin.com/4JJkhDbS would do that in an ugly and quick way
[18:05:03 CET] <kierank> having one buffer reduces the problem by a factor of a few thousand
[18:14:20 CET] <kierank> I'm pretty sure this code unscapes all NALs in the buffer n times as well
[18:19:52 CET] <nevcairiel> It should stop at the end of the NAL, but if anything that code is extremely similar to the old one. And if it does, even better, you can score performance points
[18:20:53 CET] <kierank> I also don't understand what stops the parser from leaking
[18:20:59 CET] <kierank> if nal_type isn't one of the ones in the switch
[18:21:41 CET] <kierank> nal.type
[18:22:40 CET] <kierank> ah it falls through
[18:24:02 CET] <nevcairiel> The buffer is reused to the end of the packet and then free'ed
[18:24:55 CET] <kierank> I also want to try and get rid of that never ending buffer of nal structure
[18:25:16 CET] <kierank> afaik it's all done serially
[18:26:42 CET] <kierank> maybe not possible in the current design
[18:26:43 CET] <kierank> eugh
[18:26:50 CET] <kierank> because it's factored out
[18:27:14 CET] <kierank> ok whatever as long as rbsp_buffer is fixed
[18:28:54 CET] <kierank> it used to do while(foo) { read_nal(), do_stuff()}, now it does read_nals() while(nal--){ do_stuff } :(
[18:29:00 CET] <kierank> decode_nal_units
[18:29:39 CET] <kierank> ugly ugly ugly because that structure can grow indefinitely
[18:32:44 CET] <nevcairiel> Refactoring that back might still be possible reusing the same structs we have, not sure how hevc uses that tho
[19:07:06 CET] <kierank> nevcairiel: is that place in your patch the only place where "reclaming" the rbsp buffer is allowed
[19:07:37 CET] <kierank> I gues yes
[22:42:44 CET] <durandal_1707> lets call vote on hwaccel thing!
[22:44:43 CET] <jamrial> sure
[22:45:17 CET] <jamrial> if you can summarize the issue and both sides of the argument, feel free to send an email to start a vote
[22:51:34 CET] <marsrover> durandal_1707 - is hwaccel the thing that caused wm4 to fork ffmpeg?
[23:14:33 CET] <BBB> atomnuker: hows the ffav1 decoder going? :-p
[23:16:55 CET] <kierank> BBB: are you able to look at a patch for me?
[23:17:03 CET] <kierank> patch to h264
[23:18:57 CET] <BBB> sure
[23:21:12 CET] <kierank> https://www.irccloud.com/pastebin/qobTcW9d/
[23:21:19 CET] <kierank> it works for single thread but it doesn't for multithread
[23:28:58 CET] <kierank> michaelni: do you have any thoughts on my patch?
[23:29:35 CET] <BBB> uhm& oh that stuff, no I dont know that very well, sorry :(
[23:29:47 CET] <BBB> Im only familiar with the actual bitstream bits etc., not the fluff around it
[23:30:27 CET] <kierank> :(
[23:33:40 CET] <BBB> sorry
[23:33:56 CET] <BBB> is it frame or tile threading that breaks?
[23:34:20 CET] <kierank> asan says there are problems in single thread it seems
[23:34:30 CET] <BBB> thats troubling
[23:34:31 CET] <kierank> so I'm doing something bad apparently
[23:36:08 CET] <kierank> can asan show where something was freed?
[23:38:16 CET] <jamrial> valgrind does, at least
[23:38:37 CET] <jamrial> on use after free cases
[23:40:05 CET] <kierank> valgrind is too slow
[23:44:47 CET] <jamrial> as long as it catches one segfault and tells you what freed the buffer...
[23:44:54 CET] <kierank> valgrind is too slow
[23:44:56 CET] <jkqxz> Use valgrind with a hwaccel?  That code all still runs, I think.
[23:45:03 CET] <kierank> no
[23:45:06 CET] <kierank> software h264 decoding
[23:45:18 CET] <kierank> I think that approach i took won't work
[23:45:18 CET] <kierank> eugh
[23:45:42 CET] <kierank> then i dunno what to do now apart from revert to the old code
[00:00:00 CET] --- Fri Nov  3 2017