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

[burek]

[01:06:53 CEST] <cone-985> ffmpeg 03Muhammad Faiz 07master:d535e0c14004: avcodec/pthread_frame, decode: allow errors to happen on draining
[13:01:27 CEST] <cone-556> ffmpeg 03Paul B Mahol 07master:8dd3a53dbf7d: avfilter/af_crystalizer: add support for more sample formats
[14:25:53 CEST] <cone-556> ffmpeg 03Michael Niedermayer 07master:7796f2906533: libswscale/tests/swscale: Fix uninitialized variables
[14:25:54 CEST] <cone-556> ffmpeg 03Michael Niedermayer 07master:a9b5b6a97f4f: tools: Eliminate codec_type complexity from fuzzer
[15:20:32 CEST] <durandal_1707> i found brutefir, where i could find coeffs stuff for it?
[15:21:09 CEST] <durandal_1707> going back to implementing generic fir convolver filter in lavfi
[15:21:42 CEST] <durandal_1707> anybody knows something about this topic?
[15:26:00 CEST] <cone-556> ffmpeg 03Anton Khirnov 07master:c2fa6bb0e870: mpeg12dec: move setting first_field to mpeg_field_start()
[15:26:01 CEST] <cone-556> ffmpeg 03James Almer 07master:d61f93bf04eb: Merge commit 'c2fa6bb0e8703a7a6aa10e11f9ab36094416d83f'
[17:37:52 CEST] <jamrial> nevcairiel: there, decoupled hevc sei
[17:40:24 CEST] <nevcairiel> not as terrible as it initially seemed?
[17:40:40 CEST] <jamrial> yeah, it was fairly trivial
[18:04:29 CEST] <durandal_1707> nobody?
[18:14:37 CEST] <Compn> durandal_1707 ?
[18:14:47 CEST] <Compn> [09:20] <durandal_1707> i found brutefir, where i could find coeffs stuff for it?
[18:14:47 CEST] <Compn> [09:21] <durandal_1707> going back to implementing generic fir convolver filter in lavfi
[18:14:47 CEST] <Compn> [09:22] <durandal_1707> anybody knows something about this topic?
[18:15:02 CEST] <Compn> have to ask filter people
[18:15:04 CEST] <Compn> ubitux maybe? 
[18:15:20 CEST] <Compn> probably everyone taking the weekend off, durandal_1707 :D
[18:16:04 CEST] <wm4> normal people don't work on weekends
[18:17:51 CEST] <j-b> except when it rains
[18:18:38 CEST] <jkqxz> Normal people don't work on ffmpeg, either.
[18:19:02 CEST] <alevinsn> what's normal?
[18:19:20 CEST] <j-b> what's work?
[18:19:26 CEST] <alevinsn> jamrial:  you there?
[18:25:16 CEST] <jamrial> yes
[18:27:24 CEST] <alevinsn> jamrial:  I haven't gotten through all the e-mails, so I'm not sure if you've submitted those two patches
[18:27:32 CEST] <alevinsn> but, I seem to recall that the reason you submitted them in the first place
[18:27:42 CEST] <alevinsn> was you investigated the crash my memory leak "fix" addressed
[18:28:02 CEST] <alevinsn> and you noticed that in order to fix the crash, it was necessary to copy the coded_side_data
[18:28:24 CEST] <alevinsn> by eliminating that from the patch, why bother with doing anything?
[18:28:48 CEST] <alevinsn> the function is deprecated
[18:29:16 CEST] <alevinsn> I meant, I'm not sure if you've committed those two patches
[18:29:21 CEST] <jamrial> the patchset i sent prevents your patch from crashing, even if it doesn't copy coded_side_data anymore
[18:29:26 CEST] <jamrial> i have not
[18:31:05 CEST] <alevinsn> because it zeroes out coded_side_data and nb_coded_side_data after doing the memcpy()?
[18:33:58 CEST] <alevinsn> yes, I think that's the reason
[18:34:39 CEST] <alevinsn> jamrial:  the behavior now is technically different though, and even though avcodec_copy_context() is deprecated, if any code was dependent on
[18:34:59 CEST] <alevinsn> coded_side_data being valid if it was set in the context in which it was copied from
[18:35:02 CEST] <alevinsn> that won't work anymore
[18:35:24 CEST] <alevinsn> the original code was wrong, since both AVCodecContext object's referred to the same coded side data
[18:35:35 CEST] <jamrial> exaclty, it didn't work before either. it was copying the pointer but not the buffer
[18:35:42 CEST] <alevinsn> true
[18:35:50 CEST] <alevinsn> but, perhaps because of the way the memory way freed
[18:35:51 CEST] <jamrial> what i did was do a proper clean up. i'm not going to make it copy anything else
[18:35:56 CEST] <alevinsn> it didn't cause a crash
[18:36:12 CEST] <alevinsn> and both objects "owned" the same coded side data before, which was wrong
[18:36:29 CEST] <alevinsn> but, perhaps there is some code that works on the copied context that expects coded side data to be valid
[18:36:33 CEST] <alevinsn> and that won't work anymore
[18:36:55 CEST] <alevinsn> I guess it depends on how ffmpeg.c uses that new context
[18:38:16 CEST] <alevinsn> there are multiple places that use avcodec_copy_context() unfortunately it seems
[18:41:57 CEST] <wm4> probably better to try to remove those uses, and putting effort into making avcodec_copy_context() do whatever
[18:44:53 CEST] <alevinsn> this whole coded_side_data stuff seems to be solely used by ffmpeg.c
[18:44:58 CEST] <alevinsn> I don't see any uses of it other than there
[18:45:09 CEST] <alevinsn> I'm not sure if it is even used in practice
[18:45:49 CEST] <alevinsn> I mean, there is code to populate it
[18:46:01 CEST] <alevinsn> but nothing that I've found that actually uses it
[18:47:38 CEST] <wm4> ffmpeg.c is just 1 API user
[18:47:43 CEST] <wm4> out of maybe... hundreds?
[18:48:29 CEST] <alevinsn> the way it is used seems to be via ff_add_cpb_side_data()
[18:48:33 CEST] <alevinsn> and the return value of that function
[18:52:19 CEST] <alevinsn> it seems that it is a little odd the way it is used
[18:52:40 CEST] <alevinsn> as far as I can tell, ffmpeg.c copies the coded side data into the stream
[18:52:57 CEST] <alevinsn> and then it is used via the stream with av_stream_get_side_data()
[18:53:09 CEST] <alevinsn> other than originally being populated by codecs
[18:53:26 CEST] <alevinsn> in the AVCodecContext, I don't think it is ever used, at least in the ffmpeg code base
[18:53:56 CEST] <alevinsn> and, I wouldn't be surprised if it is was added for this purpose and this purpose alone
[18:53:59 CEST] <alevinsn> to be copied into the stream
[18:54:03 CEST] <alevinsn> by ffmpeg.c
[18:54:36 CEST] <wm4> not sure
[18:55:23 CEST] <wm4> one thing is clear though: libavformat is not supposed to access the AVStream.codec ctx
[18:56:58 CEST] <alevinsn> yes, but, the whole process of copying the coded_side_data into side data for a stream
[18:57:11 CEST] <alevinsn> is a bit kludgy
[18:57:16 CEST] <alevinsn> it seems to me
[18:58:08 CEST] <wm4> yes
[19:00:21 CEST] <jamrial> both ff_add_cpb_side_data() and coded_side_data came from libav
[19:00:38 CEST] <jamrial> so they are unrelated to our temporary dependency on AVStream.codec
[19:09:10 CEST] <alevinsn> jamrial:  just sent e-mail to the list regarding those two patches--I think they are good for check-in
[19:21:29 CEST] <alevinsn> I was revisiting a patch that switched from using strncpy() to av_strlcpy() today
[19:21:44 CEST] <alevinsn> and I examined the implementation of av_strlcpy()
[19:21:46 CEST] <alevinsn> here it is:
[19:21:47 CEST] <alevinsn> size_t av_strlcpy(char *dst, const char *src, size_t size)
[19:21:47 CEST] <alevinsn> {
[19:21:47 CEST] <alevinsn>     size_t len = 0;
[19:21:47 CEST] <alevinsn>     while (++len < size && *src)
[19:21:47 CEST] <alevinsn>         *dst++ = *src++;
[19:21:49 CEST] <alevinsn>     if (len <= size)
[19:21:51 CEST] <alevinsn>         *dst = 0;
[19:21:53 CEST] <alevinsn>     return len + strlen(src) - 1;
[19:21:55 CEST] <alevinsn> }
[19:21:57 CEST] <alevinsn> Am I missing something here, or is the check
[19:22:04 CEST] <alevinsn> if (len <= size)
[19:22:08 CEST] <alevinsn> unnecessary?
[19:22:25 CEST] <alevinsn> I would think because of the way the while() loop is structured
[19:22:35 CEST] <alevinsn> than len will always be less than or equal to size by the time it gets there
[19:23:09 CEST] <alevinsn> and, if somehow it is not, then we have a problem, since the copied string may not be null-termianted
[19:23:11 CEST] <alevinsn> terminated
[19:24:59 CEST] <nevcairiel> the loop doesnt copy null termination, so instead its added there explicitly
[19:26:21 CEST] <alevinsn> i understand that
[19:26:38 CEST] <alevinsn> but, the if check seems to be unnecessary
[19:26:48 CEST] <nevcairiel> if you remove it, shit breaks
[19:26:57 CEST] <nevcairiel> since null termination would be missing
[19:27:00 CEST] <alevinsn> I'm not saying remove the null character set
[19:27:03 CEST] <alevinsn> I'm saying, do this
[19:27:50 CEST] <alevinsn> size_t av_strlcpy(char *dst, const char *src, size_t size)
[19:27:50 CEST] <alevinsn> {
[19:27:50 CEST] <alevinsn>     size_t len = 0;
[19:27:50 CEST] <alevinsn>     while (++len < size && *src)
[19:27:50 CEST] <alevinsn>         *dst++ = *src++;
[19:27:51 CEST] <alevinsn>     *dst = 0;
[19:27:53 CEST] <alevinsn>     return len + strlen(src) - 1;
[19:27:55 CEST] <alevinsn> }
[19:31:33 CEST] <nevcairiel> calling it with size = 0 is valid and it should not modify the buffer then
[19:31:34 CEST] <jkqxz> It shouldn't write anything if size is zero.
[19:31:38 CEST] <jkqxz> That.
[19:32:06 CEST] <jkqxz> It might be clearer if that was a shortcut at the beginning, though.
[19:33:02 CEST] <nevcairiel> the implementation is likely taken from somewhere else, its specifically documented to be equivalent to BSD strlcpy
[19:39:45 CEST] <alevinsn> if size is 0
[19:40:00 CEST] <alevinsn> yeah, it will work properly because it always does ++len
[19:44:07 CEST] <alevinsn> jkqxz:  I was at NAB show last week, and I spoke with Mark Buxton, who is the head of the group that works on the Intel Media SDK
[19:44:20 CEST] <alevinsn> he told me that someone on his team maintains an ffmpeg git tree
[19:44:26 CEST] <alevinsn> that has QSV improvements
[19:44:38 CEST] <alevinsn> I asked him why these changes aren't being contributed to ffmpeg proper
[19:44:50 CEST] <alevinsn> and he said because it takes too long for patches to make it in
[19:44:53 CEST] <nevcairiel> i dont think we have ever gotten one direct submission for qsv from intel
[19:45:00 CEST] <nevcairiel> so doubtful they have any data
[19:45:12 CEST] <nevcairiel> they employed some other company once to work on the qsv code
[19:45:15 CEST] <nevcairiel> but they really made it worse
[19:45:33 CEST] <alevinsn> hmm, the name of the person he mentioned is someone who works for Intel
[19:45:37 CEST] <alevinsn> I'm pretty sure
[19:45:54 CEST] <alevinsn> but, I haven't seen the git repository yet
[19:46:18 CEST] <nevcairiel> if a developer is reponsive to feedback he gets, it usually doesnt take long to get something in
[19:46:24 CEST] <nevcairiel> if they send a patch once and expect it to land
[19:46:26 CEST] <nevcairiel> well yeah
[19:46:34 CEST] <nevcairiel> corporate culture often conflicts with oss development
[19:47:11 CEST] <alevinsn> possible, although Intel, at least with certain projects, has a good track record with OSS
[19:47:56 CEST] <alevinsn> I'll see if I can track down the git repository and the person at Intel
[19:50:53 CEST] <wm4> nevcairiel: was the libyami stuff from intel devs?
[19:51:30 CEST] <nevcairiel> maybe
[19:51:39 CEST] <nevcairiel> but yet another wrapper is not an improvement to qsv code =p
[19:51:58 CEST] <jkqxz> Patches for qsv in ffmpeg tend to be from random companies using it commercially for specific things.  That tends not to be very useful, because they are attached to their use-case and don't want to do any work for anything else around it.
[19:53:42 CEST] <jkqxz> Intel hasn't done anything directly for a while, as far as I know.  Maxym Dmytrychenko has sent some stuff in the past from an Intel address, but hasn't been active recently.
[19:53:53 CEST] <jkqxz> libyami came directly from Intel, yes.
[19:55:08 CEST] <alevinsn> I have the name of the person at Intel at home that supposedly maintains the ffmpeg fork with "better" QSV code
[19:55:20 CEST] <alevinsn> I'll check that out later
[19:55:52 CEST] <nevcairiel> if they provide binaries they also have to provide sources
[19:56:36 CEST] <alevinsn> "provide binaries"--what do you mean?  there would be no point to providing ffmpeg.exe, or something like that
[19:56:55 CEST] <nevcairiel> well what do they maintain a ffmpeg fork if they dont publish it in some form somewhere
[19:57:14 CEST] <alevinsn> supposedly it is published somewhere
[19:57:23 CEST] <alevinsn> but I did a basic search just now and can't find it
[19:58:06 CEST] <alevinsn> at least, that's what Mark Buxton (the directory of Media Development Products at Intel) told me
[19:58:28 CEST] <alevinsn> but, I guess he could be mistaken, although he's pretty sharp
[19:58:44 CEST] <jkqxz> Would be interesting to see that - cleaner qsvenc support would be good for everyone.
[19:59:59 CEST] <alevinsn> jkqxz:  You suggested that I post the QSV patch to libav just because that there would be more people with QSV experience that would see it, right?
[20:00:26 CEST] <jkqxz> Yes.  Did you look at the merge I posted two (?) days ago?  I'll push that if you're happy with it.
[20:00:29 CEST] <wm4> jkqxz: would you really expect it to become "cleaner"
[20:00:55 CEST] <jkqxz> wm4:  Query things properly so you don't have the ridiculous catch all "bad rate control parameters" error.
[20:01:06 CEST] <alevinsn> wm4:  I used to work for Intel :-) .  It depends on the developer
[20:01:08 CEST] <jkqxz> That would be cleaner.
[20:01:38 CEST] <ubitux> jamrial: are you looking at dropping the hevc decoder dependency from the parser?
[20:01:50 CEST] <nevcairiel> proper runtime feature selection would be nice
[20:01:57 CEST] <wm4> the problem with these things is really that ffmpeg develoipers don't know enough about qsv, and intel developers don't know enough about ffmpeg
[20:02:03 CEST] <nevcairiel> instead of kinda hoping it matches whatever hardware we made it for
[20:02:05 CEST] <jamrial> ubitux: i tried that, yes, but fate-hevc-bsf-mp4toannexb "fails"
[20:02:07 CEST] <wm4> and the result is a misery of bad hacks
[20:02:17 CEST] <alevinsn> Intel has a paper that talks about using QSV and ffmpeg
[20:02:18 CEST] <jamrial> which is to say, the output is different, and seemingly worse
[20:02:26 CEST] <alevinsn> although, that doesn't mean they know the codebase
[20:02:41 CEST] <jkqxz> If it's adding terrible hacks to optimise some particular qsvdec->qsvenc case then that would be less interesting.
[20:02:53 CEST] <alevinsn> actually, a few papers
[20:03:12 CEST] <jkqxz> (qsvdec is dead.  Long live dxva2/d3d11va/vaapi.)
[20:03:55 CEST] <ubitux> jamrial: if you succeed, it will mean one less entry in doc/libav-merge.txt ;)
[20:04:03 CEST] <jamrial> whatever happened with that huge d3d11 patchset by Steve Lhomme?
[20:04:19 CEST] <wm4> jamrial: I picked it up, mangled it through a meat grinder, and recently reposted it
[20:04:20 CEST] <jkqxz> wm4 has adopted it, kindof.
[20:04:21 CEST] <alevinsn> jkqxz:  Perhaps the only real issue I have with the ffmpeg patches is the log messages--my original log messages
[20:04:25 CEST] <jamrial> wm4: ah cool
[20:04:31 CEST] <alevinsn> made it clear that I was pretty much restoring the code back to how it used to work
[20:04:32 CEST] <alevinsn> for ffmpeg
[20:04:35 CEST] <wm4> (libav-devel only)
[20:04:44 CEST] <alevinsn> although, I made a few changes on top of that, but it was mostly restoring things
[20:05:33 CEST] <alevinsn> Luca rewrote my log messages
[20:06:30 CEST] <jamrial> ubitux: i sent a patchset some time ago to remove the api breaking behavior from our aac_adtstoasc bsf implementation that will get rid of another libav-merge.txt line, for that matter
[20:06:36 CEST] <jamrial> hopefully we can remove lines faster than we add them :p
[20:06:41 CEST] <alevinsn> jkqxz:  but the patches are good otherwise
[20:07:07 CEST] <ubitux> jamrial: we have to keep up with the 400+ commits to merge too :(
[20:07:37 CEST] <jkqxz> alevinsn:  Right, yeah.  Do you want to write something for ffmpeg to match the patches as they are now, or shall I have a go?
[20:08:00 CEST] <alevinsn> jkqxz:  Based on what Michael wrote about merges last week
[20:08:16 CEST] <alevinsn> it seems that he prefers it if developers submit new patches to ffmpeg for review
[20:08:26 CEST] <alevinsn> rather than it being "merged" in
[20:08:32 CEST] <alevinsn> so, perhaps I ought to do that
[20:09:40 CEST] <jkqxz> Sure.  If you want to send it to the ML then I can commit it from there.  (Not like anyone else is ever going to say anything about it on ffmpeg-devel, which was why I suggest libav instead.)
[20:09:40 CEST] <alevinsn> since you've already reviewed the changes, it should be a simple matter to commit them as patches instead of merges
[20:10:15 CEST] <alevinsn> but, there does see to be a preference, at least from Michael, to commit as regular patches rather than merges
[20:10:23 CEST] <jkqxz> To clarify, I believe that merge comment was really about developers merging changes to things that they are not responsible for.
[20:10:24 CEST] <alevinsn> if the developer is so inclined to submit to both MLs
[20:10:44 CEST] <alevinsn> oh
[20:10:52 CEST] <jkqxz> Oh, maybe that wasn't quite the same thing.
[20:11:05 CEST] <alevinsn> it seemed fairly general what Michael was talking about
[21:35:40 CEST] <jamrial> ubitux: guess what :D
[23:19:37 CEST] <jamrial> ubitux, nevcairiel: if you could spare a review or two, that'd be great :p
[00:00:00 CEST] --- Mon May  1 2017