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

[burek]

[00:01:20 CEST] <durandal_1707> cehoyos: why?
[00:02:19 CEST] <cone-633> ffmpeg 03Takayuki 'January June' Suwa 07master:f9a061a31c3d: avdevice/alsa: fix indefinite stop on closing PCM capture
[00:03:08 CEST] <cehoyos> Why I was asking for aac? Or why I closed the ticket where it is claimed that either Dolby or DTS are "not normal" (don't remember which)?
[00:03:40 CEST] <cehoyos> There is already a ticket for the stupid resampling for dts -> ac3
[00:03:49 CEST] <cehoyos> (a "new team" regression)
[00:03:57 CEST] <cehoyos> sorry, a paid new team regression
[00:04:29 CEST] <durandal_1707> frei0r
[00:04:34 CEST] <cehoyos> ;-)
[00:04:48 CEST] <cehoyos> The patch that fixes the warning is wrong afaict, I sent a new patch.
[00:05:04 CEST] <cehoyos> But I could not really test so maybe I miss a lot...
[00:06:28 CEST] <cehoyos> Please review
[00:06:47 CEST] <durandal_1707> fetch frei0r and test it?
[00:08:13 CEST] <cehoyos> I did and I can reproduce the original issue (before the patch last year) and the warning but frei0r crashes here elsewhere, but since my patch should not change the actual argument to frei0r, I don't think that really matters.
[00:08:30 CEST] <cehoyos> The warning fix was just wrong imo.
[00:09:04 CEST] <durandal_1707> where it crashes?
[00:10:04 CEST] <cehoyos> Sorry, I removed the binary: An assertion failure in frei0r
[00:11:21 CEST] <cehoyos> Did you look at my patch?
[00:12:22 CEST] <durandal_1707> no, its not in  mail as txt but attached as binary
[00:12:58 CEST] <durandal_1707> tomorrow,now sleep time
[00:13:05 CEST] <cehoyos> Good night!
[00:20:02 CEST] <durandal_1707> patch looks fine, will test tomorrow
[00:22:58 CEST] <cehoyos> Please do, thank you
[01:41:49 CEST] <cone-633> ffmpeg 03Cyber Sinh 07master:499b46fd0a89: compat/windows/makedef: Allow building shared libs with MSVC under WSL
[01:43:37 CEST] <cone-633> ffmpeg 03Carl Eugen Hoyos 07master:d0ca749adbf2: tests/fate-run: New variable hostexecsuf for local fate tools.
[17:08:10 CEST] <cone-882> ffmpeg 03Michael Niedermayer 07master:caa9b4ff89c0: avcodec/agm: Check that there is available input in read_code()
[17:08:10 CEST] <cone-882> ffmpeg 03Michael Niedermayer 07master:fee666104519: avcodec/dsicinvideo: check the amount decoded by cin_decode_huffman()
[17:08:10 CEST] <cone-882> ffmpeg 03Michael Niedermayer 07master:9570322a2d8c: avcodec/dxtory: Check slice_size against minimum in dxtory_decode_v2()
[17:08:10 CEST] <cone-882> ffmpeg 03Michael Niedermayer 07master:ed188f6dcdf0: avformat/aadec: Check for scanf() failure
[17:08:10 CEST] <cone-882> ffmpeg 03Michael Niedermayer 07master:18a567c369d7: avformat/mov: Skip stsd adjustment without chunks
[17:08:10 CEST] <cone-882> ffmpeg 03Michael Niedermayer 07master:6f0e9a863466: avutil/avstring: Fix bug and undefined behavior in av_strncasecmp()
[17:08:10 CEST] <cone-882> ffmpeg 03Michael Niedermayer 07master:8b10f09fd537: avcodec/arbc: Skip unchanged frames
[17:08:11 CEST] <cone-882> ffmpeg 03Michael Niedermayer 07master:7c2ee8d43d95: avcodec/arbc: Try to correct keyframe/frame type
[17:10:45 CEST] <philipl> BtbN: so the nvenc thing? should we just make the change and document the assumption?
[17:16:27 CEST] <jamrial> jkqxz: there's a bunch of cbs patches from andreas and some by me, if you can look whenever you have time
[18:08:07 CEST] <BtbN> philipl, I looked into reverting the relevant patch, but it needs more indepth changes to do things properly.
[18:08:30 CEST] <BtbN> And I haven't found time to do so yet.
[18:10:33 CEST] <philipl> Bok
[18:12:01 CEST] <BtbN> There are two patches which basically need reverted again, so there is only a global list of mappings, and not one per frame
[18:14:55 CEST] <BtbN> http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=bbe1b21022e4872bc64066d46a4567dc1b655f7a;hp=a026a3efaeb9c2026668dccbbda339a21ab3206b see commit message as to why it can't just be reverted.
[18:18:03 CEST] <BtbN> ok, it's not as complicated. Unfortunately, I can't test if it works, cause I'm not seeing any performance regression at all.
[18:21:58 CEST] <philipl> I assume these guys will let us know :-)
[18:27:08 CEST] <BtbN> Did you see the patch about the nvidia h264 parser getting interlaced flags wrong?
[18:49:26 CEST] <philipl> BtbN: I totally believe it.
[18:49:52 CEST] <BtbN> It's easily reproducible (dude that sent the patch prepared half a dozen docker containers for some reason)
[18:50:03 CEST] <BtbN> Run these in order to see!
[18:50:17 CEST] <BtbN> And before that do these 12 commands to make using GPUs in Docker possible in the first place
[18:50:27 CEST] <philipl> I did not think hard, but doesn't this mean that mixed interlacing streams will have problems because we are making a stream level decision?
[18:50:55 CEST] <BtbN> I don't think so, cause the sequence callback will be re-called for those I believe.
[18:51:50 CEST] <BtbN> Which is also why I moved the location where it's set from the original patch to the display callback
[18:52:13 CEST] <BtbN> to make sure an intermediate re-call of the sequence callback does not make it use the new flag on old frames still in the queue
[18:55:45 CEST] <BtbN> Where would you even document special behaviour of an encoder that only an API user would care about?
[18:56:46 CEST] <BtbN> nvenc does not have an entry in encoders.texi. Maybe it should?
[19:03:08 CEST] <JEEB> yea
[19:03:30 CEST] <BtbN> philipl, I'm tempted to just #ifdef out the part that unregisters the frames. That way someone who really relies on the behavour can easily build a version that does so.
[19:05:09 CEST] <BtbN> Could also turn it into a normal option, but it seems a bit arcane to expose that way.
[19:05:24 CEST] <BtbN> Is there a way to make options hidden?
[19:06:59 CEST] <BtbN> doesn't seem like it
[19:07:47 CEST] <philipl> BtbN: we discussed making it an option when it first came up. I'd be ok with it. Others might not be.
[19:08:19 CEST] <philipl> It's only relevant for a very customised application usage pattern. Documenting it as such seems to be fine.
[19:20:43 CEST] <BtbN> philipl, https://github.com/BtbN/FFmpeg/commit/5f77951908677d1f9a0f27bb19ebaa6cb567900f
[19:22:11 CEST] <nevcairiel> why not have fully compatible method be default, and the performance edge-case require special compilation?
[19:24:55 CEST] <nevcairiel> potential leaks are worse then slowdown in extreme special cases
[19:32:02 CEST] <philipl> because there's no way to directly use ffmpeg that requires the compatible path
[19:32:15 CEST] <philipl> You need a custom pool allocator
[19:32:23 CEST] <nevcairiel> people use APIs as well you know
[19:32:38 CEST] <nevcairiel> and such an option is pretty well hidden
[19:32:55 CEST] <philipl> Of course, but the edge case, in practice, the non-performant one.
[19:32:58 CEST] <philipl> that's what I mean.
[19:33:59 CEST] <philipl> When I say directly use, I mean you must provide a custom allocator that doesn't re-use a fixed pool to need the aggressive unregistration.
[19:34:07 CEST] <philipl> You can't combine off-the-shelf parts to create that situation.
[19:35:12 CEST] <nevcairiel> you would be surprised how many people just grab zeranoe windows builds to use the .dlls because they cant build their own
[19:35:28 CEST] <nevcairiel> and even if you do compile yourself, finding and knowing about that option is still quite a step
[19:36:49 CEST] <philipl> Oh, I thought it was a runtime option.
[19:36:57 CEST] <philipl> I'm seeing now it's not
[19:37:58 CEST] <nevcairiel> it could be i suppose, but even then the option thats not error prone in some situations is usually favorable over performance optimizations
[19:38:21 CEST] <nevcairiel> ideally you would be able to detect this from the pool or something
[19:47:04 CEST] <BtbN> nevcairiel, it's not a leak though
[19:47:15 CEST] <BtbN> The resources are eventually unregistered
[19:47:31 CEST] <BtbN> Just not immediately, but at the end of the session, of if the buffer is full
[19:48:05 CEST] <nevcairiel> so what actually happens if i use new buffers for every frame?
[19:48:49 CEST] <BtbN> The undefined situation happens when you free the CUdevptr that nvenc still has registered
[19:49:21 CEST] <BtbN> as long as you just keep feeding in an endless stream of new handles it will eventually run out of buffer space to store them, free one, and register there.
[19:49:34 CEST] <nevcairiel> obviously i would relesae those frames
[19:49:43 CEST] <BtbN> What happens then is up to the nvidia driver.
[19:50:02 CEST] <BtbN> If it implicitly unregisters them, and doesn't complain when trying to do so again, all will be fine
[19:51:10 CEST] <BtbN> But in what situation would you not keep re-using the same set of input surfaces over and over again?
[19:52:10 CEST] <BtbN> Keep in mind that the behaviour I'm restoring right now was the default in nvenc for the longest time, and nobody ever had issues.
[19:54:36 CEST] <BtbN> I'm just gonna try doing that, it's an easy test
[19:59:37 CEST] <BtbN> nevcairiel, how do I free whatever d3d11va gives to nvenc in frame->data[0]?
[19:59:42 CEST] <nevcairiel> this only affects cuda input, not software or d3d?
[19:59:56 CEST] <BtbN> Software input does not care, d3d11va is equally affected
[20:01:50 CEST] <nevcairiel> ID3D11Texture2D_Release(frame->data[0]) i think
[20:02:06 CEST] <BtbN> yeah, looking at the hwcontext stuff for it, seems like that's it
[20:02:45 CEST] <nevcairiel> not sure if it needs a cast
[20:02:46 CEST] <nevcairiel> maybe
[20:02:47 CEST] <BtbN> I have just thrown that in right after registering a frame, and then try to unregister it again right away
[20:02:51 CEST] <BtbN> ID3D11Texture2D_Release((ID3D11Texture2D *)frame->data[0]);
[20:03:14 CEST] <nevcairiel> the way d3d11 frames are allocated its even less l ikely to get that problem tho
[20:03:52 CEST] <BtbN> If the unregister succeeds (or throws a well defined error like "not registered"), there is no need for the special flag in the first place
[20:04:02 CEST] <BtbN> cause that can be handled and safely ignored
[20:07:20 CEST] <nevcairiel> thinking about it, d3d11 is a bad test, since there is only one ref-counted texture object with X layers, so it'll never  actually be free'ed
[20:09:52 CEST] <BtbN> Yeah, d3d11 has those sub layers
[20:27:31 CEST] <BtbN> it works just fine if I free the thing. Unregisters without errors
[20:27:35 CEST] <BtbN> for d3d11
[20:27:38 CEST] <BtbN> testing CUDA now
[20:28:42 CEST] <BtbN> Yeah, works just fine as well.
[20:41:17 CEST] <philipl> BtbN: so you can unregister after free in practice?
[20:41:36 CEST] <BtbN> it does not throw an error at least
[20:41:50 CEST] <philipl> heh. Ship it.
[20:47:11 CEST] <BtbN> I guess it would explode if you were to actually re-use it
[20:58:06 CEST] <philipl> BtbN: I don't get the first_round thing
[20:59:30 CEST] <BtbN> It avoid unregistering frames if there are still slots without a registered frame int hem
[20:59:34 CEST] <BtbN> *in them
[21:00:02 CEST] <BtbN> Cause right now, if it's looking for an unmapped spot, it'll just unregister whatever is in there
[21:00:28 CEST] <BtbN> And not prefer fully empty ones
[21:01:08 CEST] <philipl> ah, ok.
[21:05:04 CEST] <BtbN> though in reality, it'll never reach that function again after a short while, because all input surfaces will be registeres already
[21:10:54 CEST] <philipl> sure.
[22:57:25 CEST] <cone-887> ffmpeg 03Paul B Mahol 07master:e1cfb01b054b: avfilter/af_surround: fix typo
[22:57:25 CEST] <cone-887> ffmpeg 03Paul B Mahol 07master:e1e0f94dc956: avfilter/af_surround: add angle option
[22:57:25 CEST] <cone-887> ffmpeg 03Paul B Mahol 07master:2d16b83824c1: avfilter/af_surround: check for invalid magnitude and phase difference
[22:57:25 CEST] <cone-887> ffmpeg 03Paul B Mahol 07master:604421630bd5: avfilter/af_surround: improve rear channels separation
[23:36:41 CEST] <cone-887> ffmpeg 03James Almer 07master:53cc3338f7b2: avcodec/h264_ps: fix storage size for offset_for_ref_frame
[23:36:42 CEST] <cone-887> ffmpeg 03James Almer 07master:a42e761b9623: avcodec/h264_ps: use get_se_golomb_long() to parse some sps fields
[00:00:00 CEST] --- Thu Apr 25 2019