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

[burek]

[00:13:33 CEST] <BtbN> "no mention of the poor Ryzen performance." I'm not aware of any performance issues on Ryzen, what is that guy talking about?
[00:13:45 CEST] <BtbN> (talking about the 4.2 changelog)
[00:15:37 CEST] <kierank> BtbN: crappy benchmarks
[00:15:43 CEST] <kierank> most benchmarks are useless
[00:15:49 CEST] <kierank> just noise in power saving throttling
[00:16:06 CEST] <BtbN> ffmpeg benchmarks?
[00:19:08 CEST] <Lord_Nightmare> forgive me if this question is stupid, but: in https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/adpcm.c around line 229, there's two functions for returning the next 16-bit signed PCM value when fed with an IMA ADPCM nybble, and the two functions are almost identical
[00:19:38 CEST] <Lord_Nightmare> is there a reason there are separate functions, rather than the first one calling the second, or both calling a common function?
[00:20:39 CEST] <TD-Linux> I don't actually know how to lock ryzen clocking yet. I have a dedicated cannonlake box for that purpose right now
[00:21:04 CEST] <BtbN> You set the governor to performance I'd guess?
[00:21:12 CEST] <BtbN> On Windows you just change the power plan to Max Performance
[00:21:26 CEST] <BtbN> But Generally Windows drives their clocks just fine
[00:22:01 CEST] <kierank> BtbN: cpu can still go into c-states
[00:22:16 CEST] <kierank> most of the benchmarks on phoronix use files
[00:22:19 CEST] <kierank> and other such nonsense
[00:23:50 CEST] <BtbN> If it's on tmpfs, it should be fine
[00:25:55 CEST] <durandal_1707> Lord_Nightmare: please mention functions you are talking about
[00:26:13 CEST] <Lord_Nightmare> sorry
[00:26:38 CEST] <Lord_Nightmare> static inline int16_t adpcm_ima_expand_nibble(ADPCMChannelStatus *c, int8_t nibble, int shift)
[00:26:48 CEST] <Lord_Nightmare> static inline int16_t adpcm_ima_wav_expand_nibble(ADPCMChannelStatus *c, GetBitContext *gb, int bps)
[00:27:26 CEST] <Lord_Nightmare> in the latter case, it has a GetBitContext pointer since presumably the .wav variant of ima adpcm allows nybbles of a size other than 4 bits
[00:27:35 CEST] <durandal_1707> later one supports different bps adpcm variants
[00:27:36 CEST] <BtbN> They do look like one could be implemented using the other
[00:27:44 CEST] <Lord_Nightmare> i need to check the IMA ADPCM spec document
[00:27:55 CEST] <Lord_Nightmare> the only significant difference i see is the way delta is calculated
[00:28:04 CEST] <durandal_1707> and one is faster than other one
[00:28:32 CEST] <BtbN> How hot is that function?
[00:28:44 CEST] <durandal_1707> very
[00:29:32 CEST] <durandal_1707> if fuzz are being speed up why not this one actually used code
[00:30:05 CEST] <Lord_Nightmare> i'm trying to think of some way that the code on lines 244-252 and on lines 267-275, which is 100% identical, could be reduced to appearing just once, without adding another function unless it would be compiled inline in both cases without wasting extra register/stack operations
[00:30:35 CEST] <Lord_Nightmare> i guess it could be done as a common #define IMA_SOMETHING but that's sort of ugly
[00:30:44 CEST] <durandal_1707> you are thinking too much imho
[00:30:52 CEST] <TD-Linux> both amd and intel have various other reclocking unrelated to cpu governor (recent ones actually have a hardware governor and don't really use the linux one at all)
[00:31:20 CEST] <TD-Linux> on my intel machine have BIOS options to disable most of it
[00:31:52 CEST] <TD-Linux> (which is why it's a dedicated machine - if I had to reboot every time to run checkasm it'd be obnoxious)
[00:32:00 CEST] <durandal_1707> if you find nicer way to implement it, feel free to send patch
[00:32:36 CEST] <Lord_Nightmare> http://www.cs.columbia.edu/~hgs/audio/dvi/IMA_ADPCM.pdf is the original ima adpcm spec, which stupidly doesn't define the endianness for packing 4-bit adpcm nybbles to a byte
[00:32:54 CEST] <Lord_Nightmare> i think some codecs do it one way and some do the other
[00:33:14 CEST] <durandal_1707> yes, quite possible
[00:34:15 CEST] <kierank> BtbN: they arent
[00:34:28 CEST] <durandal_1707> the other code covers thing not covered with complete specs
[00:34:46 CEST] <Lord_Nightmare> another thing that the ima adpcm spec document does not do is define where the values in that 89-entry step size table actually come from
[00:34:55 CEST] <Lord_Nightmare> its a simple calculation
[00:35:32 CEST] <durandal_1707> yes?
[00:35:41 CEST] <Lord_Nightmare> IMA ADPCM is basically an expanded/enhanced version of the old OKI adpcm from the MSM5205 adpcm decoder IC, which hias its own codec called OKI or Dialogic ADPCM
[00:35:51 CEST] <Lord_Nightmare> durandal_1707: one sec
[00:35:58 CEST] <Lord_Nightmare> I have some code for it, I'll pastebin it
[00:37:53 CEST] <Lord_Nightmare> wait, its just 5 lines
[00:37:57 CEST] <Lord_Nightmare> should i paste it directly?
[00:38:15 CEST] <Lord_Nightmare> er, 6 lines
[00:38:32 CEST] <durandal_1707> pastebin
[00:38:36 CEST] <Lord_Nightmare> ok
[00:39:15 CEST] <Lord_Nightmare> recompiling to make sure it has the correct results, I wrote this 2 years ago
[00:43:47 CEST] <Lord_Nightmare> easier to just dump the file on dropbox: https://www.dropbox.com/s/lcjx0umwxur37dm/stepgen_ima.c?dl=0
[00:44:35 CEST] <durandal_1707> not for me
[00:44:41 CEST] <Lord_Nightmare> one liner would be int stepval = min(floor(16.0 * pow(11.0 / 10.0, (double)(step-8))), 32767); // where step is 0 thru 88
[00:45:17 CEST] <BtbN> That's several magnitudes slower
[00:45:25 CEST] <Lord_Nightmare> that just generates the table
[00:45:50 CEST] <Lord_Nightmare> i'm not proposing doing that every sample
[00:46:35 CEST] <Lord_Nightmare> also the values that puts out are very slightly wrong; that equation was used to generate the original table, then two entries were tweaked by hand before the ima thing. I think.
[00:47:23 CEST] <durandal_1707> maybe, also there is already oki decoder
[00:47:25 CEST] <Lord_Nightmare> the output of that equation is "7,8,9,9,10,12,13,14,..." while the IMA spec is "7,8,9,10,11,12,13,14,..."
[00:47:35 CEST] <Lord_Nightmare> the rest of the numbers match exactly
[00:48:43 CEST] <Lord_Nightmare> if you wanted to generate the table once, on startup (rather than using a pre-initialized array), that's the only time I can think where that code would make sense
[00:48:56 CEST] <Lord_Nightmare> otherwise its of academic interest only
[00:49:15 CEST] <durandal_1707> hmm, i would check reference binary
[00:49:36 CEST] <durandal_1707> of thing firdt doing it
[00:50:25 CEST] <Lord_Nightmare> interestingly i found out that there's two different oki decoding specs: the msm5205 did not clamp the accumulator to a 12 bit range, so it would wrap if it overflowed or underflowed; the later oki chips all clamp the range
[00:51:18 CEST] <Lord_Nightmare> interestingly, the yamaha ym2610 ADPCM_A codec, which is a modified variant of msm5205, uses the msm5205 behavior where overflowing/underflowing is allowed, and many samples on the neogeo (gunshot in metal slug and others) take advantage of this
[00:52:10 CEST] <Lord_Nightmare> yamaha ym2610 ADPCM_B codec however is 100% identical to the .wav "Yamaha ADPCM" codec 
[00:52:40 CEST] <Lord_Nightmare> which is a different type of adpcm (Jayant instead of Oki)
[00:53:07 CEST] <durandal_1707> yea, there is huge amount of adpcm variants
[00:53:08 CEST] <Lord_Nightmare> though i suppose you could say that oki is an optimized variant of jayant where there's only one multiply instead of 2...
[00:56:36 CEST] <durandal_1707> https://news.ycombinator.com/item?id=20629325
[00:57:31 CEST] <durandal_1707> troll spotted there...
[01:03:15 CEST] <jkqxz> tmm1:  I would weakly prefer the CBS A/53 and related stuff set polished and finished off to putting it into VAAPI as you have in that patch.
[01:03:37 CEST] <jkqxz> tmm1:  Though I haven't looked at that set in a long time.
[02:48:04 CEST] <cone-168> ffmpeg 03Michael Niedermayer 07n4.3-dev:HEAD: Revert "avformat/rtpdec_rfc4175: Fix incorrect copy_offset calculation"
[04:09:03 CEST] <Compnn> Lord_Nightmare, is your change binary identical output with all ima adpcm samples? :D
[09:07:59 CEST] <Lord_Nightmare> Compnn: i haven't made any changes yet (I don't have an ffmpeg build environment set up anyhow; i'm using mingw32-w64 on windows)
[10:00:33 CEST] <cone-289> ffmpeg 03Ruiling Song 07master:98e419cbf544: avfilter/vf_convolution: add x86 SIMD for filter_3x3()
[12:07:34 CEST] <mkver> Do we actually have a place for patches that should only be applied after an ABI break?
[12:09:57 CEST] <rcombs> mkver: you apply the patch but have the breaking part behind FF_API_[whatever], and test by bumping the major version locally
[12:10:54 CEST] <mkver> Ok. And what should be put in the APIChanges file?
[12:16:47 CEST] <BtbN> The api change
[12:21:52 CEST] <nevcairiel> most ABI-incompatible changes are removals, so the APIChanges would get the deprecation or whatever of the old thing
[12:24:12 CEST] <mkver> I actually meant what date I should put into the APIChanges file: The current date or leave a placeholder for the actual date of the version bump.
[12:25:37 CEST] <mkver> My ABI-incompatible change is changing avio_enum_protocols: From const char * (void **opaque, int output) to const char * (const void **opaque, int output).
[12:57:40 CEST] <nevcairiel> we've had const additions before, just check what they did
[16:01:57 CEST] <cone-654> ffmpeg 03Linjie Fu 07master:af3ddd581faf: lavf/vf_vpp_qsv: add support for QSV transpose filter
[17:10:02 CEST] <vel0city> is there a fast way to update patch versions? atm I'm passing --annotate to send-email and changing each commit's subject line one by one every time
[17:18:54 CEST] <jkqxz> vel0city:  git format-patch -v2 ...
[17:25:00 CEST] <vel0city> @jkqxz: ahh, thanks!
[20:27:31 CEST] <mkver> ff_h264dsp_init() contains these lines:     if (ARCH_AARCH64) ff_h264dsp_init_aarch64(c, bit_depth, chroma_format_idc);
[20:27:31 CEST] <mkver>     if (ARCH_ARM) ff_h264dsp_init_arm(c, bit_depth, chroma_format_idc);
[20:27:31 CEST] <mkver>  and also some more for other architectures. No #if, just if. These functions are all declared in h264dsp.h, but the definition is in arch-specific files and some of them won't be compiled. What if a compiler does not remove the dead code during compilation? Wouldn't linking fail?
[20:27:50 CEST] <J_Darnley> Yes
[20:28:16 CEST] <J_Darnley> Which is why we require dead code elimination (if that's the term)
[20:29:42 CEST] <J_Darnley> I think msvc used to be one that didn't do it if you enabled some debugging options.
[20:30:44 CEST] <durandal_1707> some people come once in a while with patches to fix compilers that does not support dead code elimination
[20:32:53 CEST] <nevcairiel> full debug doesnt work with MSVC indeed, but DCE is used in so many places that we just gloss over that
[20:33:02 CEST] <nevcairiel> i usually just enable debug for the files i currently work on
[21:31:23 CEST] <mkver> michaelni: "awnser" in your most recent commit message.
[22:36:16 CEST] <durandal_1707> michaelni: drop your leadership role, will you?
[22:43:12 CEST] <michaelni> durandal_1707, iam not leader, and please stop these attacks against me. 
[22:45:03 CEST] <durandal_1707> not attacking at all
[22:46:35 CEST] <durandal_1707> im purely request for new ffmpeg leadership, when one is ignored to join security mailing list
[22:47:01 CEST] <jamrial> you're being aggressive, attack or not
[22:47:01 CEST] <nevcairiel> noone cares for your complaints
[22:47:22 CEST] <jamrial> there are better ways to complain or be heard. there's no need to be aggressive
[22:47:36 CEST] <durandal_1707> and when security is run by single person completely
[22:48:01 CEST] <jamrial> there's more than one person in the security email alias
[22:48:39 CEST] <durandal_1707> i see only one person do the actual paid work
[22:48:40 CEST] <jamrial> two years ago there was drama about it, and the list of people being forwarded the emails was made public
[22:52:23 CEST] <durandal_1707> that one person constantly sends bunch of patches per day,  some of questionable quality
[22:53:47 CEST] <durandal_1707> also that single person holds bunch of infrastructure stuff
[22:57:09 CEST] <nevcairiel> except there is more then one person on the security mailing list
[22:57:29 CEST] <durandal_1707> irrelevant
[22:57:31 CEST] <nevcairiel> and personally, i would not vote for you  for security, if i'm being honest
[22:57:57 CEST] <durandal_1707> thanks for your honesty
[22:59:21 CEST] <kierank> lol
[23:14:29 CEST] <durandal_1707> and since when this leader had sent some useful addition like implementing non trivial new feature to ffmpeg?
[23:18:27 CEST] <kierank> durandal_1707: integer overflows are important
[23:19:03 CEST] <durandal_1707> these are the most benign patches of all
[23:19:24 CEST] <durandal_1707> except ugly to look at
[23:19:30 CEST] <kierank> a lot of the runtime ones are ridiculous I agree
[23:19:59 CEST] <kierank> the one which counts iterations of an infinite loop is horrible
[23:23:31 CEST] <kierank> some of them are good patches though
[23:23:43 CEST] <durandal_1707> why some people claim that ffmpeg maintenance is so producing bad quality
[23:24:01 CEST] <kierank> ?
[23:24:41 CEST] <durandal_1707> look at that ycombinator posts
[23:25:04 CEST] <durandal_1707> bad ugly legacy non maintained code
[23:32:41 CEST] <J_Darnley> durandal_1707: if you want a new leader nominate me for HDFL
[23:34:11 CEST] <durandal_1707> minister of human development and family life?
[23:34:33 CEST] <J_Darnley> hostile dictator for life
[23:36:34 CEST] <durandal_1707> also i fixed numerous security issues, pro bono
[23:36:36 CEST] <kierank> Link?
[23:38:23 CEST] <kierank> Oh god
[23:38:27 CEST] <kierank> I read that thread
[23:38:35 CEST] <kierank> j-b: oh god that hn thread
[23:58:15 CEST] <nevcairiel> why do people read hn, its  such a cesspool these days
[00:00:00 CEST] --- Thu Aug  8 2019