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

burek burek021 at gmail.com
Thu Mar 23 03:05:03 EET 2017


[00:23:39 CET] <cone-847> ffmpeg 03James Almer 07release/2.8:a1ad585c8ef1: avformat/apng: fix setting frame delay when max_fps is set to no limit
[00:23:40 CET] <cone-847> ffmpeg 03James Almer 07release/3.0:48706b9fef0c: avformat/apng: fix setting frame delay when max_fps is set to no limit
[00:23:41 CET] <cone-847> ffmpeg 03James Almer 07release/3.1:b014fa21d4a3: avformat/apng: fix setting frame delay when max_fps is set to no limit
[00:23:42 CET] <cone-847> ffmpeg 03James Almer 07release/3.2:33978a49c025: avformat/apng: fix setting frame delay when max_fps is set to no limit
[00:44:12 CET] <cone-847> ffmpeg 03Marton Balint 07master:51546504133c: avcodec/avcodec.h: clarify decoupled decode/encode API docs
[02:02:03 CET] <cone-847> ffmpeg 03Jun Zhao 07master:9365dfcbf665: hwcontext: fix comments for av_hwdevice_ctx_alloc()
[02:27:33 CET] <cone-847> ffmpeg 03Matthias Hunstock 07master:607bffbed287: avdevice/decklink: add format_code of display mode to list_format output
[02:27:34 CET] <cone-847> ffmpeg 03Matthias Hunstock 07master:b3a2adaac652: avdevice/decklink: new option 'format_code' to set video format by fourCC
[04:20:40 CET] <cone-847> ffmpeg 03James Almer 07master:aee046a895db: x86/audiodsp: remove an unnecessary movss
[06:26:47 CET] <IRC-Source_62632> I have a question, can ffserver stream audio in PCM raw format?
[07:49:14 CET] <wm4> jkqxz: ported the vdpau patch to ffmpeg and sent to ML
[07:49:36 CET] <wm4> the conflict was trivial, but there were other problems (legacy bullshit that wasn't in Libav caused crashes)
[07:50:10 CET] <wm4> I tested the normal vdpau API and the hw_frames_ctx, but not the old vdpau API, the very old vdpau API, or the hw_device_ctx API
[08:53:16 CET] <kierank> damn, should have got a gsoc student to write memory leak testsa
[08:56:53 CET] <kierank> not sure if it's a gsoc worth of work though
[10:35:11 CET] <Hawoo_> Hi:) I am a student from GSoC program. I would like to participate in VMAF video filter program but the specified qualification task was already completed, I wonder is there any alternative task I can choose? Thanks a lot!
[10:40:13 CET] <nevcairiel> Best would be to contact one of the listed mentors directly
[11:07:04 CET] <cone-829> ffmpeg 03Steven Liu 07master:8ddadf56f621: avformat/rtmpproto: change rtmp_open from url_open to url_open2
[11:25:39 CET] <ubitux> fate is broken :(
[11:25:55 CET] <ubitux> (apng stuff)
[11:30:33 CET] <cone-829> ffmpeg 03Justin Ruggles 07master:43717469f9da: ac3dsp: Reverse matrix in/out order in downmix()
[11:30:34 CET] <cone-829> ffmpeg 03Clément BSsch 07master:e39d4ff150f4: Merge commit '43717469f9daa402f6acb48997255827a56034e9'
[11:31:53 CET] <wm4> what broke it?
[11:40:46 CET] <cone-829> ffmpeg 03Justin Ruggles 07master:a9ba59591ed5: ac3dsp: Add some special-case handling for the C downmix function
[11:40:47 CET] <cone-829> ffmpeg 03Clément BSsch 07master:fd5e1d132b46: Merge commit 'a9ba59591ed509fb7e6decfde8da4cbfd4ddf4b8'
[11:43:19 CET] <cone-829> ffmpeg 03Clément BSsch 07master:ce10e4cb1f21: doc/libav-merge: create a special "extra changes" section
[11:47:39 CET] <jkqxz> wm4:  VDPAU is surely fine, but it's probably worth checking on Mesa before you push.  I'll do that later today.  (I assume you're using Nvidia blob.)
[11:48:38 CET] <cone-829> ffmpeg 03Clément BSsch 07master:9dc57688c8e2: lavc/mips: temporally disable ac3 downmix
[11:50:34 CET] <wm4> jkqxz: yes, nvidia blob here
[11:50:49 CET] <wm4> jkqxz: well, feel free to push my patches once you've confirmed it on mesa
[12:48:11 CET] <BBB> does anyone have more ideas for vmaf qualification tasks?
[12:48:19 CET] <wm4> so, x264 has commercial licensing, right? then I don't understand why the ffmpeg x264 support requires GPL? unless the wrapper itself is explicitly GPL licensed
[12:48:21 CET] <BBB> or alternatively, is any gsoc mentor looking for more students?
[12:52:15 CET] <RiCON> wm4: and you can build a lgpl x264 too
[12:53:12 CET] <wm4> wut
[12:53:19 CET] <ubitux> huh? :o
[12:53:30 CET] <ubitux> for real?
[12:55:09 CET] <cone-829> ffmpeg 03Justin Ruggles 07master:b57e38f52cc3: ac3dsp: x86: Replace inline asm for in-decoder downmixing with standalone asm
[12:55:10 CET] <cone-829> ffmpeg 03Clément BSsch 07master:c66bd8f3ff28: Merge commit 'b57e38f52cc3f31a27105c28887d57cd6812c3eb'
[12:55:19 CET] <RiCON> --disable-gpl            disable GPL-only features
[12:55:25 CET] <RiCON> or does this do nothing?
[12:55:53 CET] <ubitux> so we could have a ffmpeg+libx264 build in LGPL?
[12:56:49 CET] <BBB> I dont think so
[12:57:04 CET] <wbs> RiCON: isn't that more that you can avoid including any gpl-only feature in a commercially licensed build
[12:57:06 CET] <BBB> configure urrently dies if gpl is disabled but x264 is enabled
[12:57:30 CET] <BBB> and also what wbs said
[12:57:32 CET] <wbs> RiCON: like, not trying to include gpl dependencies if you've got a commercial license
[12:57:45 CET] <BBB> there is no lgpl x264
[12:57:49 CET] <BBB> there is only commercial or gpl
[12:57:57 CET] <wm4> anyway, this was just from a nitpicky, theoretical point of view, since I suspect most --enable-gpl GPL violations by companies were done for the x264 wrapper
[12:57:57 CET] <BBB> disable-gpl is for commercial builds
[12:58:01 CET] <ubitux> ok ok
[12:58:09 CET] <wm4> and these companies could just license x264 commercially
[12:58:42 CET] <BtbN> So if a company has a commercial license, the enable-gpl build of ffmpeg, if it's only for x264, is ok?
[12:58:52 CET] <wm4> dunno lol
[12:59:47 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:2124711b950b: hwcontext_vaapi: add a quirk for the missing MemoryType attribute
[12:59:48 CET] <cone-829> ffmpeg 03Clément BSsch 07master:4205010240fe: Merge commit '2124711b950b03c582a119c75f52a87acc32d6ec'
[13:07:05 CET] <RiCON> hm, --disable-gpl doesn't actually seem to do anything other than change Copyleft to Copyright and license to "Non-GPL commercial"
[13:07:20 CET] <RiCON> there's no gpl-only filters or files
[13:07:43 CET] <nevcairiel> for x264? no, its either GPL or commercial license
[13:07:49 CET] <nevcairiel> you need to buy the second
[13:08:06 CET] <ubitux> pthread_mutex_unlock failed with error: Operation not permitted
[13:08:08 CET] <ubitux> zsh: abort (core dumped)
[13:08:10 CET] <ubitux> meeeh
[13:08:12 CET] <ubitux> :(
[13:09:58 CET] <nevcairiel> wm4 wanted to look into that
[13:10:37 CET] <nevcairiel> and yeah apng-clock seems broken
[13:10:42 CET] <nevcairiel> maybe just needs a ref update
[13:11:26 CET] <ubitux> ok, i'll wait for the next merge then
[13:19:04 CET] <wm4> uh yeah
[13:19:24 CET] <wm4> yesterday it seemed like something strange is going on rather than a simple logic error
[13:59:13 CET] <ubitux> wm4: "current thread does not own the mutex"?
[14:02:33 CET] <wm4> ooh, that could be
[14:02:40 CET] <wm4> maybe it's indeed unlocked by another thread
[14:17:51 CET] <jamrial> ubitux: we'll have to disable the mips ac3dec downmix code since it wont compile or work anymore
[14:17:54 CET] <jamrial> since we can't fix it
[14:18:29 CET] <jamrial> the mips guys will have to adapt it. and maybe also add those 5 to 1/2 channel downmix paths to the fixed decoder
[14:18:32 CET] <ubitux> i did in a later commit
[14:18:35 CET] <ubitux> and mailed the guy
[14:18:46 CET] <jamrial> oh, missed the separate commit
[14:18:48 CET] <jamrial> cool then
[14:18:51 CET] <ubitux> yeah i realized later
[14:19:06 CET] <ubitux> jamrial: btw, you need to fix the apng fate test
[14:19:15 CET] <ubitux> :p
[14:19:32 CET] <jamrial> ah fuck
[14:19:45 CET] <jamrial> thanks for reminding me :p
[14:28:37 CET] <cone-829> ffmpeg 03James Almer 07master:0dbfed08d06a: fate: update ref file for apng-clock test
[14:37:07 CET] <DHE> I have a patch I'm considering submitting. Looking for more of a "does this patch make sense?" than a review: https://github.com/DeHackEd/FFmpeg/commit/472dddaf97418e00
[14:37:32 CET] <DHE> short version: if an mpeg-ts file appears corrupted due to continuity check errors, it shouldn't be a DEBUG level log event, it should be a WARNING.
[14:38:19 CET] <DHE> and I use the word "file" loosely
[14:38:20 CET] <atomnuker> adding an option to do that isn't good, just report it as warning by default
[14:38:50 CET] <DHE> yeah, I thought about that. question of is it worth changing current behaviour or add an option and maintain existing behaviour
[14:39:01 CET] <wbs> DHE: 7e75f061825dd29059ae3110814c8e424aa1e2e2
[14:39:26 CET] <DHE> oh geez...
[14:40:27 CET] <wbs> (iirc, in practice, one gets that warning on lots of hls streams, if the mpegts stream in one segment doesn't match the previous segment)
[14:41:16 CET] <wbs> and there are hls streams where you lose packets in the gap between segments if you close and reopen the demuxer, so to handle them, you need to actually treat the segments as one continuous stream
[14:41:38 CET] <BBB> wm4: sorry I didnt notice you already lgtmed elsewhere
[14:42:04 CET] <DHE> that... actually makes sense..
[14:42:40 CET] <wbs> DHE: so perhaps your patch, changing the default to warning, but have the hls demuxer set it back to debug?
[14:43:41 CET] <wm4> lgmt'ed what? doesn't matter much anyway
[15:01:07 CET] <ubitux> michaelni: opinion on 5bf2454e7cb? we decode that sample fine in FFmpeg (but logging is flooded with errors in our case, even with the patch applied)
[15:02:15 CET] <ubitux> basically, applying the patch has no effect because we simply don't error out and enter this code
[15:03:42 CET] <ubitux> (our ff_h2645_packet_split() doesn't fail)
[15:14:58 CET] <wm4> ubitux: yeah, you're right, it unlocks the mutex on a different thread
[15:15:34 CET] <wm4> curiously it doesn't normally happen
[15:15:37 CET] <wm4> only with ffplay
[15:15:54 CET] <nevcairiel> ubitux: we support such samples already, just differently
[15:15:59 CET] <nevcairiel> (re 5bf2454e7cb)
[15:16:45 CET] <nevcairiel> wm4: it seems rather fragile in general if some order of api calls can abort the process :d
[15:17:01 CET] <wm4> nevcairiel: it's just UB
[15:17:10 CET] <ubitux> nevcairiel: yes we do, but with lots of errors in the logging, so maybe the patch from Libav is better
[15:17:26 CET] <ubitux> but maybe the errors are unrelated
[15:17:30 CET] <wm4> possibly it's because ffplay calls APIs from different threads or so (which it is allowed to)
[15:17:58 CET] <wm4> I really don't like what the code does with async_mutex anyway
[15:18:14 CET] <nevcairiel> ubitux: the libav approach would also still log an error for every packet before it re-tries parsing as annexb
[15:18:42 CET] <ubitux> but we have like 5-6 for every frame
[15:19:00 CET] <ubitux> see the sample from https://github.com/HandBrake/HandBrake/issues/339
[15:24:44 CET] <nevcairiel> hm those are not parsing errors
[15:24:53 CET] <nevcairiel> but actual decoding errors
[15:25:03 CET] <nevcairiel> so unless it mis-reads some things due ot parsing problems
[15:25:20 CET] <nevcairiel> do we decode the same stuff from it as libav after the patch?
[15:25:54 CET] <nevcairiel> generally we log quite a bunch more error conditions then libav does
[15:28:39 CET] <ubitux> but the errors i see are present in libav codebase
[15:42:53 CET] <nevcairiel> ubitux: the frame hashes are identical, so not sure its worth worrying about
[15:44:12 CET] <ubitux> ok well, i'll skip it then
[15:44:28 CET] <ubitux> any idea what commit may have fix that issue? (for hash ref in merge description)
[15:45:47 CET] <nevcairiel> 93b89868e139e9b45dfc8a62b4f8e1832bbfd5d8 i would guess
[15:45:56 CET] <nevcairiel> it handles the same case in mp4, but should apply to avi equally
[15:48:47 CET] <ubitux> indeed, i confirm commenting out that chunk makes it fail
[15:48:52 CET] <ubitux> thanks
[15:50:15 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:5bf2454e7cb0: h264dec: support broken files with mp4 extradata/annex b data
[15:50:16 CET] <cone-829> ffmpeg 03Clément BSsch 07master:17f2c0abea87: Merge commit '5bf2454e7cb03609b3ec1a3cf4c22427fe5f8e36'
[15:50:49 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:d10102d23c94: avconv: set the encoding framerate when the output is CFR
[15:50:50 CET] <cone-829> ffmpeg 03Clément BSsch 07master:97c17cafab4e: Merge commit 'd10102d23c9467d4eb84f58e0cd12be284b982f6'
[15:54:13 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:f6772e9bf825: avconv: make sure the filtergraph is freed on init failure
[15:54:14 CET] <cone-829> ffmpeg 03Clément BSsch 07master:204b4a7ee2e9: Merge commit 'f6772e9bf8251d3943f52f6f34d97d2ce6c4b8af'
[16:05:52 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:27085d1b47c3: avconv: only retry decoding on actual decoding errors
[16:05:53 CET] <cone-829> ffmpeg 03Clément BSsch 07master:928db5134478: Merge commit '27085d1b47c3741cc0fac284c916127c4066d049'
[16:09:47 CET] <michaelni> ubitux, \(One\ Piece\ AMV\)\ Centuries.avi decodes with far fewer errors in ffmpeg-3.0 than later
[16:10:02 CET] <michaelni> i mean log errors
[16:10:14 CET] <ubitux> ok, so probably unrelated
[16:15:56 CET] <cone-829> ffmpeg 03Clément BSsch 07master:c7904af05723: lavc/huffyuvdsp: remove unused ppc init prototype
[16:23:09 CET] <cone-829> ffmpeg 03Clément BSsch 07master:af607b7e0787: lavc/huffyuvdsp: only transmit the pix_fmt instead of the whole avctx
[16:26:36 CET] <ubitux> oh actually we split these optims out
[16:32:42 CET] <cone-829> ffmpeg 03Alexandra Hájková 07master:22c3ab186469: checkasm: Add test for huffyuvdsp add_bytes
[16:32:43 CET] <cone-829> ffmpeg 03Clément BSsch 07master:7c2a7f9c11ab: Merge commit '22c3ab18646924ce24dc6017a9e882ff69689e40'
[16:34:02 CET] <stevenliu> Hello
[16:34:04 CET] <stevenliu> Rodger Combs here
[16:34:13 CET] <RiCON> he's rcombs
[16:34:20 CET] <rcombs> ohai
[16:34:32 CET] <stevenliu> Hi, This is Steven Liu
[16:34:51 CET] <stevenliu> I want talk and learn about the av_asprintf :)
[16:35:21 CET] <stevenliu> I saw libavformat/segment.c use it to build file name too 
[16:35:59 CET] <stevenliu> Is that mean it need to modify for looks like replace operation?
[16:36:09 CET] <rcombs> you mean hls.c?
[16:36:42 CET] <rcombs> or, actually, that doesn't use it either
[16:36:44 CET] <cone-829> ffmpeg 03Luca Barbato 07master:1d25a8690294: huffyuvdsp: Reenable PPC optimizations
[16:36:46 CET] <cone-829> ffmpeg 03Clément BSsch 07master:7a11e6b2fde8: Merge commit '1d25a86902946dbc80bb3a38e61755181ca3af7b'
[16:36:47 CET] <rcombs> which file and line are you referring to
[16:37:30 CET] <stevenliu> 279     snprintf(seg->temp_list_filename, sizeof(seg->temp_list_filename), seg->use_rename ? "%s.tmp" : "%s", seg->list);
[16:37:39 CET] <stevenliu> libavformat/segment.c
[16:38:00 CET] <rcombs> oh, snprintf
[16:38:02 CET] <cone-829> ffmpeg 03Luca Barbato 07master:b015872c0d08: huffyuvdsp: Enable the altivec code for PPC little-endian as well
[16:38:04 CET] <cone-829> ffmpeg 03Clément BSsch 07master:34389e847213: Merge commit 'b015872c0d0823e70776e98b865509ec1287e2f6'
[16:38:09 CET] <stevenliu> Is that wrong?
[16:38:13 CET] <rcombs> stevenliu: OK, notice that the format string argument there is a string constant
[16:38:22 CET] <rcombs> "%s.tmp" or "%s" depending on seg->use_rename
[16:38:26 CET] <rcombs> that's fine
[16:38:46 CET] <rcombs> the problem with your code is that the format string argument comes from the XML file
[16:39:05 CET] <rcombs> you should never use dynamic data like that as the format-string argument to a function in the printf family
[16:39:11 CET] <rcombs> it's a security vulnerability
[16:39:46 CET] <rcombs> it can result in memory corruption, leading potentially to crashes or code execution, if the format string is maliciously crafted
[16:39:49 CET] <ubitux> simple example of why: put "%s%s%s%s%s%s%s%s%s%s%s%s%s" in your xml
[16:39:54 CET] <rcombs> ^
[16:40:03 CET] <stevenliu> 222     snprintf(seg->cur_entry.filename, size, "%s%s",
[16:40:16 CET] <rcombs> %s will probably crash; %p can cause code execution
[16:40:24 CET] <rcombs> stevenliu: that's writing _into_ seg->cur_entry.filename
[16:40:27 CET] <stevenliu> No, not come from XML file
[16:40:30 CET] <rcombs> the first arg to snprintf is the output
[16:40:37 CET] <stevenliu> it is compte from XML Duration
[16:40:48 CET] <rcombs> "%s%s" is the format string there
[16:41:44 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:13f5d2bf75b9: configure: check for stdatomic.h
[16:41:45 CET] <cone-829> ffmpeg 03Clément BSsch 07master:2d6f7e6c74bd: Merge commit '13f5d2bf75b95a0bfdb9940a5e359a719e242bed'
[16:41:55 CET] <rcombs> stevenliu: it's set in rep->url_template = replace_template_str(temp_string, "$Number$");, and temp_string comes from temp_string = get_content_url(baseurl_nodes, 4, rep_id_val, rep_bandwidth_val, media_val);
[16:42:25 CET] <rcombs> get_content_url gets its base string from the XML file (xmlNodeGetContent)
[16:42:38 CET] <rcombs> and then replaces some template strings
[16:42:40 CET] <jamrial> ubitux: just ported the ac3 fixed downmix functions
[16:42:52 CET] <stevenliu> 1197             int64_t tmp_val = pls->tmp_url_type == TMP_URL_TYPE_NUMBER ? pls->cur_seq_no : get_fragment_start_time(pls, pls->cur_seq_no);
[16:43:07 CET] <jamrial> the ones in ac3dsp. i can't find a sample that uses the one in ac3dec_fixed to test
[16:43:23 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:4e928ef340ac: Add a compat stdatomic.h implementation based on GCC atomics
[16:43:24 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:c2755864afad: Add a compat stdatomic.h implementation based on windows atomics
[16:43:25 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:bb81ed476569: Add a compat stdatomic.h implementation based on suncc atomics
[16:43:26 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:f9a6a80e065c: Add a compat stdatomic.h implementation based on pthreads
[16:43:27 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:eb34d40354e2: Add a compat dummy stdatomic.h used when threading is disabled
[16:43:28 CET] <cone-829> ffmpeg 03Clément BSsch 07master:f015711ed124: Merge commit 'eb34d40354e2474517c9b9bd787e0dadc89c2a81'
[16:43:35 CET] <rcombs> stevenliu: that's not the format string
[16:43:43 CET] <rcombs> the format string is pls->url_template (the first arg to av_asprintf())
[16:43:46 CET] <stevenliu> yes , it is val
[16:43:56 CET] <ubitux> jamrial: did you see the int16 version?
[16:44:01 CET] <stevenliu> url_template is replaced 
[16:44:09 CET] <jamrial> i said above i couldn't find a sample for it :p
[16:44:15 CET] <stevenliu> looks like %lld
[16:44:30 CET] <ubitux> jamrial: oh, ok sorry :)
[16:44:37 CET] <ubitux> (thanks!)
[16:45:10 CET] <kkanungo17> hi folks!
[16:45:18 CET] <ubitux> i actually did it locally pre-split, but couldn't test so ignored
[16:45:34 CET] <ubitux> i guess you just added the int64_t case and "+x>>y"
[16:45:35 CET] <rcombs> stevenliu: right, but the base string that gets valued replaced to produce url_template comes from the XML
[16:45:46 CET] <jamrial> ubitux: yeah. and any ac3 sample works
[16:45:47 CET] <ubitux> and s/cmp_matrix/matrix/
[16:45:51 CET] <jamrial> even the fate test i added
[16:46:01 CET] <ubitux> alright, cool
[16:46:12 CET] <ubitux> so far LGTM, but i'm not a maintainer of that code
[16:46:23 CET] <stevenliu> get from XML and get from user set have different?
[16:46:56 CET] <ubitux> no
[16:47:20 CET] <rcombs> I don't understand the question
[16:47:47 CET] <stevenliu> get from XML, have been replaced insert %lld to replace $$, set the template from user by command line, i think is same thing
[16:48:16 CET] <ubitux> in the xml, if you add %s%s%s%s in the string where there is the $$
[16:48:20 CET] <ubitux> you're going to have a bad time
[16:48:45 CET] <stevenliu> you mean, ffmpeg will have a bad format to output dash?
[16:49:03 CET] <rcombs> I still don't understand what you're trying to say
[16:49:13 CET] <rcombs> but if you don't fix this then your patch will not be accepted
[16:49:40 CET] <ubitux> stevenliu: ffmpeg will crash at best
[16:50:39 CET] <stevenliu> ubitux's anwser is accept, but rcombs' is look like dictatorship :D
[16:50:59 CET] <stevenliu> anyway, i fix it
[16:51:00 CET] <rcombs> well that's code review for you
[16:51:23 CET] <stevenliu> Not for me, just for ffmpeg
[16:51:31 CET] <ubitux> stevenliu: rcombs is right, your patch introduces a major security issue so it won't be accepted 
[16:51:43 CET] <stevenliu> ffmpeg not support it now
[16:51:52 CET] <stevenliu> just add dashdec into ffmpeg , that is it
[16:52:00 CET] <stevenliu> yes
[16:52:23 CET] <durandal_1707> yes we know
[16:52:32 CET] <stevenliu> he is right
[16:53:10 CET] <stevenliu> let me fix it :)
[16:53:57 CET] <stevenliu> Thanks rcombs ubitux and durandal, thanks guys
[16:55:00 CET] Last message repeated 1 time(s).
[17:00:04 CET] <ubitux> dammit, what's this USE_ATOMICS in our lavu/buffer.c
[17:01:47 CET] <ubitux> can we drop that now?
[17:03:41 CET] <jamrial> ubitux: 6db8cd8f37b and 890d8f44fdf
[17:03:47 CET] <jamrial> so michaelni might know
[17:05:35 CET] <ubitux> 27079a426c9d3db918b158976e44b9b143d78e1c is strange
[17:05:50 CET] <ubitux> it does move the avbuffer refcount to a "stdatomic"
[17:06:17 CET] <ubitux> or wait i missed it does it for the pool one as well
[17:06:43 CET] <ubitux> ok i get it, i should be able to update it, but that will probably be a blind one
[17:08:51 CET] <ubitux> that USE_ATOMIC code smells really bad
[17:09:32 CET] <ubitux> http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavutil/buffer.c;h=478e4880e5c604ad0686fac9444218d3e5951d6d;hb=HEAD#l388
[17:09:39 CET] <ubitux> this is full of races or i'm missing something?
[17:10:05 CET] <ubitux> doing a pool->refcount <= pool->nb_allocated, and then next line using avpriv_atomic_int_get for the error message
[17:10:07 CET] <ubitux> what's going on?
[17:11:50 CET] <jamrial> michaelni: ^
[17:12:43 CET] <wm4> ubitux: is that the code that was removed from libav
[17:13:01 CET] <wm4> yes it is
[17:13:06 CET] <wm4> ask michaelni, he committed this crime
[17:13:14 CET] <jamrial> cea3a63ba3d seems like the reason why it was added is no longer valid
[17:13:29 CET] <wm4> of course
[17:13:34 CET] <jamrial> since this code is used only on non threading builds
[17:13:43 CET] <jamrial> because of that USE_ATOMICS check
[17:14:01 CET] <ubitux> ok so that part can go away
[17:14:10 CET] <ubitux> now what about the whole USE_ATOMIC?
[17:15:14 CET] <wm4> kill it
[17:17:12 CET] <jamrial> if you kill it (enabling these atomics on builds with threading enabled) wouldn't the reason that code above was added for become valid again?
[17:20:09 CET] <ubitux> the code is in the USE_ATOMICS scope
[17:20:19 CET] <ubitux> i'm trying to remove completely what's inside these scopes
[17:21:07 CET] <rcombs> just require atomics
[17:21:16 CET] <ubitux> http://sprunge.us/YZZb
[17:22:57 CET] <ubitux> (oh and dropping nb_allocated in buffer_internal.h)
[17:24:25 CET] <ubitux> it appears the next commits in libav drops all the libavutil/atomic.h refs
[17:24:41 CET] <ubitux> (we'll have a few more)
[17:24:55 CET] <ubitux> so should i commit that and go on with the merges?
[17:26:06 CET] <michaelni> jamrial, i dont want to be impolite and not reply to being pinged but what should i say ? that it might reintroduce races and then people attack and remove it anyway ... i rather spend my day in peace
[17:26:08 CET] <wm4> ubitux: I say yes
[17:26:42 CET] <wm4> yeah because it's totally valid that people compile without threads and then expect code to be threadsafe
[17:26:44 CET] <wm4> TOTALLYX
[17:26:51 CET] <ubitux> michaelni: any objection on removing USE_ATOMICS?
[17:26:56 CET] <ubitux> wm4: please :)
[17:27:22 CET] <ubitux> i'm asking because it's blocking my merges
[17:27:27 CET] <wm4> it's nonsensical to support
[17:27:52 CET] <ubitux> maybe, i still want to understand the rationale and if it was/is/will be required
[17:28:12 CET] <wm4> no
[17:28:24 CET] <wm4> the atomics code was race so it was replaced with mutexes (in Libav)
[17:28:53 CET] <michaelni> ubitux, i didnt follow the merges and changes, i dont object to removing it if it doesnt introduce issues, if it does its maybe better to leave it
[17:29:21 CET] <ubitux> i can't tell if it introduce issues as it's being used only if... there is no thread implementation
[17:29:59 CET] <ubitux> ...but it's using various current atomic code
[17:32:22 CET] <michaelni> is there something that prevents user apps which use threads to use a lib* that was built without threads ? are we checking for this, is this documented as a problem in the previous releases ? and what if a user app is build arond a different thread implementation than the one used in the libs mutexes ?
[17:33:35 CET] <michaelni> anyway these are just comments i object to no choice for this
[17:37:52 CET] <ubitux> i think wm4 is right with saying that if you're building ffmpeg without any threads you can't except any lock mechanism
[17:38:10 CET] <ubitux> i'll check that if it still work if i disable threading
[17:38:33 CET] <ubitux> if it does i'll drop it and go on with the merges
[17:44:10 CET] <ubitux> seems to pass, RIP
[17:44:38 CET] <cone-829> ffmpeg 03Clément BSsch 07master:67d8eabdbb29: lavu/buffer: drop USE_ATOMICS
[17:49:14 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:27079a426c9d: buffer: convert to stdatomic
[17:49:15 CET] <cone-829> ffmpeg 03Clément BSsch 07master:443e9692935f: Merge commit '27079a426c9d3db918b158976e44b9b143d78e1c'
[17:49:29 CET] <ubitux> wm4: i'll merge the v4l2 commit soon, but can you have a look to the mmal one and the 3 next?
[17:49:38 CET] <ubitux> i think they're related to your pthread crash
[17:49:53 CET] <ubitux> (which affects teh progress, but the commits switch this stuff to stdatomic)
[17:55:53 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:3a165c187da7: v4l2: convert to stdatomic
[17:55:54 CET] <cone-829> ffmpeg 03Clément BSsch 07master:b7336faa39b8: Merge commit '3a165c187da7d74f46f6c1778294e8c5a3a7151f'
[17:56:46 CET] <ubitux> mmal doesn't differ much so it should be trivial to merge
[17:56:56 CET] <ubitux> for the others, it might be a bit more tricky
[17:57:30 CET] <ubitux> (it applies cleanly)
[17:58:50 CET] <ubitux> (ETA: we're close to 800 commits left)
[18:01:23 CET] <wm4> are there any conflicts in the mmal one?
[18:01:27 CET] <ubitux> nope
[18:01:31 CET] <ubitux> it applies cleanly
[18:01:33 CET] <wm4> then it should be fine
[18:02:12 CET] <wm4> which pthread crash?
[18:02:21 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:8385ba53f115: mmaldec: convert to stdatomic
[18:02:22 CET] <cone-829> ffmpeg 03Clément BSsch 07master:82d6179a88bb: Merge commit '8385ba53f115401a67a4748c0d107769ebfb2941'
[18:02:31 CET] <ubitux> wm4: the one we talked about earlier
[18:02:41 CET] <ubitux> the mutex lock from another thread
[18:02:46 CET] <wm4> the ffplay one triggers only if you enable some debug stuff (if not it'll hang or work, maybe latter)
[18:04:16 CET] <ubitux> ok well, i'll look at these commits as well
[18:06:08 CET] <wm4> I think I tried to port all commits on pthread_frame
[18:06:43 CET] <wm4> and the next commit was redundant or so
[18:08:09 CET] <wm4> anyway whether or not the commit was already merged or whatever happened, it's ok not needed in current ffmpeg master
[18:10:26 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:db2733256db3: pthread_frame: use a thread-safe way for signalling threads to die
[18:10:27 CET] <cone-829> ffmpeg 03Clément BSsch 07master:e6c868befa6a: Merge commit 'db2733256db323e4b88a34b135320f33274148e2'
[18:12:02 CET] <ubitux> ah indeed all of this is already merged
[18:12:35 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:64a31b2854c5: pthread_frame: use atomics for PerThreadContext.state
[18:12:36 CET] <cone-829> ffmpeg 03Clément BSsch 07master:5aee64991999: Merge commit '64a31b2854c589e4f27cd68ebe3bcceb915704e5'
[18:14:13 CET] <cone-829> ffmpeg 03Anton Khirnov 07master:59c70227405c: pthread_frame: use atomics for frame progress
[18:14:14 CET] <cone-829> ffmpeg 03Clément BSsch 07master:9ce031c37f13: Merge commit '59c70227405c214b29971e6272f3a3ff6fcce3d0'
[18:15:03 CET] <ubitux> we have a lot of stuff to update in our codebase to be able to drop atomic.h
[18:15:54 CET] <ubitux> anyone will to help, especially for videotoolboxenc, decklink and mediacodec which i can't test?
[18:15:59 CET] <ubitux> willing*
[18:17:05 CET] <cone-829> ffmpeg 03Clément BSsch 07master:cb763a9ba849: lavc/bitstream: remove unused atomic.h include
[18:17:50 CET] <cone-829> ffmpeg 03Clément BSsch 07master:cf1c8a379e9f: lavc/bitstream_filter: remove unused atomic.h include
[18:19:40 CET] <jamrial> ubitux: the stdatomics are pretty much drop in replacements for the atomic.h stuff, so they can probably be replaced blindly
[18:19:47 CET] <jamrial> if it doesn't work, mac people will complain anyway :p
[18:19:54 CET] <wm4> why mac people
[18:20:02 CET] <ubitux> because videotoolbox :p
[18:20:14 CET] <ubitux> but there is also windows & android :)
[18:20:21 CET] <ubitux> but sure i can blind it
[18:20:40 CET] <ubitux> i'd still prefer is someone who can test does it :p
[18:20:54 CET] <jamrial> only issue is add and fetch, which is not available on stdatomic
[18:20:58 CET] <jamrial> there's only fetch and add
[18:21:11 CET] <jamrial> but a previous merge already dealt with that
[18:21:42 CET] <atomnuker> ubitux: can I push something?
[18:22:00 CET] <ubitux> atomnuker: yes
[18:22:22 CET] <cone-829> ffmpeg 03Rostislav Pehlivanov 07master:38d7cc22f778: mdct15: fix left shift of a negative value
[18:22:34 CET] <atomnuker> ubitux: ktnx, that was it
[18:22:55 CET] <ubitux> jamrial: http://sprunge.us/GZAI does this looks good to you?
[18:24:03 CET] <ubitux> actually i need to update the type in the struct too
[18:24:05 CET] <jamrial> why init instead of store?
[18:24:14 CET] <nevcairiel> i was wondering about the type
[18:24:16 CET] <nevcairiel> :d
[18:25:18 CET] <jamrial> the compat headers don't seem to use any kind of relevant atomic function for init() but do for store()
[18:25:50 CET] <nevcairiel> init should be used in actual init code that never runs in parallel, store otherwise
[18:26:20 CET] <ubitux> ah indeed i missed the store
[18:28:15 CET] <wm4> the current code assumes 0 initializing atomics is ok anyway
[18:28:26 CET] <wm4> although in theory that's probably not the case
[18:28:44 CET] <jamrial> ubitux: is videotoolboxenc.c even using anything from lavu/atomic.h?
[18:29:04 CET] <ubitux> it appears not
[18:29:06 CET] <ubitux> :p
[18:29:17 CET] <jamrial> one down then :p
[18:29:42 CET] <ubitux> http://sprunge.us/cBhR
[18:29:48 CET] <ubitux> does this look better?
[18:31:22 CET] <ubitux> i'm not sure if i can miss the atomic_init
[18:32:58 CET] <jamrial> it worked without it before
[18:33:37 CET] <ubitux> because of the memset 0 of the context
[18:33:57 CET] <ubitux> but officially i'm supposed to init these
[18:35:28 CET] <jamrial> then leave it
[18:36:29 CET] <ubitux> what's the replacement for avpriv_atomic_ptr_cas?
[18:37:24 CET] <jamrial> atomic_compare_exchange it seems
[18:38:34 CET] <cone-829> ffmpeg 03Clément BSsch 07master:e28bd75f7cfd: lavc/hevc: use atomics for wpp_err
[18:39:19 CET] <ubitux> help welcome for fixing the remaining git grep 'include.*[^d_]atomic.h'
[18:39:26 CET] <ubitux> i'm going away for a while
[18:39:38 CET] <ubitux> as soon as they are dropped i can move on with the merges
[18:39:59 CET] <ubitux> (ETA: 799 commits)
[18:48:00 CET] <BtbN> philipl, how do you even test encoding with nvenc with b frames?
[18:48:38 CET] <BtbN> Setting -bf to anything other than 0 makes EncodePicure fail for me
[18:50:57 CET] <BtbN> Or is that the failure you mean? As it works just fine without hwaccel cuvid
[18:53:02 CET] <BBB> wm4: I dont think libav removed atomics?
[18:53:39 CET] <wm4> BBB: ?
[18:53:59 CET] <nevcairiel> they replaced our custom atomics with C11 atomics
[18:54:04 CET] <BBB> <wm4> the atomics code was race so it was replaced with mutexes (in Libav) (at 1 1/2 hr ago)
[18:54:06 CET] <BBB> right
[18:54:18 CET] <BBB> nevcairiel: indeed
[18:54:21 CET] <nevcairiel> not all of the atomics
[18:54:25 CET] <nevcairiel> but some use of it
[18:54:30 CET] <nevcairiel> was what wm4 referred to
[18:54:33 CET] <BBB> I believe they removed the custom atomics and now use stdc atomics
[18:54:34 CET] <BBB> ok
[18:55:12 CET] <BBB> if this is about pthreads, that work of wah-teng was done on ffmpeg and I asked him to do it in libav instead (after I approved his patch here) so that we wouldnt have widely diverging implementations of the same thing
[18:55:17 CET] <wm4> s/race/racy
[18:55:18 CET] <BBB> (pthread_frame, I mean)
[18:55:35 CET] <wm4> that's what ubitux is merging right now
[18:55:47 CET] <wm4> we have additional old-atomics use so these must be fixed first
[18:55:59 CET] <BBB> right
[19:07:27 CET] <philipl> BtbN: Yes. But it worked before the filter merge
[19:07:36 CET] <philipl> That was my point
[19:35:27 CET] <jamrial> ubitux, nevcairiel: http://pastebin.com/raw/PE0ZxPUe mediacodec
[22:17:35 CET] <cone-829> ffmpeg 03Diego Biurrun 07master:6ff3da4f6a8e: Place attribute_deprecated in the right position for struct declarations
[22:17:36 CET] <cone-829> ffmpeg 03Mark Thompson 07master:81b7deab8296: vaapi: Implement device-only setup
[22:17:37 CET] <cone-829> ffmpeg 03Mark Thompson 07master:14c110151838: vaapi_hevc: Mark as async-safe
[22:17:38 CET] <cone-829> ffmpeg 03Mark Thompson 07master:9560766a6164: vaapi_vp9: Mark as async-safe
[22:42:20 CET] <cone-829> ffmpeg 03James Almer 07master:30cadfe071ad: avcodec/lossless_videodsp: use ptrdiff_t for length parameters
[23:45:14 CET] <kierank> atomnuker: would you be willing to mentor aac-960 encoding by any chacne? Mathias continues to troll me
[23:45:48 CET] <nevcairiel> someone should get decoding of that going first =p
[00:00:00 CET] --- Thu Mar 23 2017


More information about the Ffmpeg-devel-irc mailing list