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

[burek]

[00:07:51 CEST] <cone-189> ffmpeg 03U. Artie Eoff 07master:f70c397456c7: vaapi_encode_mjpeg: fix bad component id bug
[00:07:52 CEST] <cone-189> ffmpeg 03Alejandro Solozabal 07master:caabe1b4956d: avcodec/omx: Correct av_log() log message
[00:18:12 CEST] <mkver> jkqxz: Why doesn't cbs use unchecked bitstream readers? After all, cbs does its own checks for overreads.
[00:24:37 CEST] <jkqxz> Mostly a preference on my part for safety over performance in that API, so I never investigated it.  I wouldn't object if you want to change it (and it makes a nontrivial difference for something you care about).
[00:25:49 CEST] <mkver> Ok, I'll benchmark it.
[00:30:55 CEST] <mkver> Btw: I think that the display orientation stuff in h264_metadata (and in h264_slice and hevcdec) is completely buggy.
[00:32:13 CEST] <mkver> The reason is that in all these instances when reading a SEI and creating the corresponding matrix, the rotation is applied first and then the flip. But according to the H.2645 standard, the flips should be performed first, then the rotation.
[00:38:00 CEST] <mkver> (The new matrix M' that corresponds to applying a flip after having applied the transformation corresponding to the matrix M is the matrix M with some of its columns multiplied by (-1). If the flip is applied first, then the new matrix is created by multiplying certain rows with (-1).)
[00:46:02 CEST] <jkqxz> I'm confused by the standard.  The first example in the note doesn't seem to make sense?
[00:46:38 CEST] <jkqxz> (In H.265 D.3.17.)
[00:50:25 CEST] <jkqxz> Unhelpfully with the second example it doesn't matter which way around the flip/rotate happens.
[00:51:16 CEST] <mkver> Yes, that note is wrong: One can't have anticlockwise_rotation equal to 0x8000000.
[00:53:06 CEST] <jkqxz> 0x8000 is just a half turn.
[00:53:48 CEST] <mkver> Yes, and that is what they meant.
[00:55:29 CEST] <jkqxz> What do you mean?  The 0x8000 is correct for the second example.
[00:56:05 CEST] <mkver> They meant this: "It is possible for equivalent transformations to be expressed in multiple ways using these syntax elements. For example, the combination of having both hor_flip and ver_flip equal to 1 with anticlockwise_rotation equal to 0 can alternatively be expressed by having both hor_flip and ver_flip equal to 0 with anticlockwise_rotation equal to 0x8000, and the combination of hor_flip
[00:56:05 CEST] <mkver>  equal to 1 with ver_flip equal to 0 and anticlockwise_rotation equal to 0 can alternatively be expressed by having hor_flip equal to 0 with ver_flip equal to 1 and anticlockwise_rotation equal to 0x8000."
[00:56:50 CEST] <mkver> I mean that they made three mistakes: They twice wrote 0x8000000 where they should have written 0x8000.
[00:57:30 CEST] <jkqxz> Are we looking at the same thing?  I see 0x8000.
[00:58:15 CEST] <mkver> And they wrote "can alternatively be expressed by having both hor_flip and ver_flip equal to 1" (that 1 should be 0).
[00:58:34 CEST] <jkqxz> Ah, that looks like it was corrected between 2015 and 2018.
[00:58:56 CEST] <jkqxz> The flip part is still wrong in 2018, though.
[00:58:59 CEST] <mkver> I se 0x8000000. I use the H.264 standard 04/2017 and H.265 12/2016.
[00:59:10 CEST] <mkver> I'll get the newer standards.
[00:59:58 CEST] <jkqxz> I'm looking at 2018/02.
[01:01:09 CEST] <mkver> Yes. That has been corrected.
[01:05:33 CEST] <mkver> The same thing (i.e. GCC 9 being more strict about the scope of compound literals) is btw behind a data corruption bug in the linux kernel (that affects users having a SSD cache + HDD combination).
[01:06:59 CEST] <mkver> And the codepath for inserting a display orientation SEI from a side-data matrix is btw. also not bug-free.
[01:08:24 CEST] <mkver> The check whether to flip is wrong: One should check at the determinant of the upper left 2x2 submatrix. If it is negative, one needs to use a flip (doesn't matter which one); if not, no flip.
[01:13:01 CEST] <jkqxz> Hmm, I don't really remember that code at all.  I was only interested in rotations, so I'm not surprised if flips are wrong somehow.
[01:16:17 CEST] <mkver> Ok. Did you manage to reproduce the infinite loop I mentioned in https://ffmpeg.org/pipermail/ffmpeg-devel/2019-June/245054.html or was I too cryptic?
[01:24:07 CEST] <jkqxz> I think the hevcdec code is doing the right thing?  It would be helpful if it had some more comments to explain the ordering, but it does have the right effect (flip then rotate).
[01:25:06 CEST] <mkver> No, it sets the rotation, then it flips.
[01:29:14 CEST] <mkver> av_display_matrix_flip is effectively multiplying the given matrix with a diagonal matrix (whose diagonal entries are +1 or -1 depending on hlip and vflip) *from the right*.
[01:30:21 CEST] <mkver> In ordinary linear algebra, the vector to which the matrix is applied is written to the right of the matrix, i.e. if we apply several matrices/linear transformations in sequence, the rightmost matrix is applied first.
[01:30:56 CEST] <jkqxz> Urgh.  I confused the rotation direction in testing it and got it the wrong way around.
[01:31:34 CEST] <mkver> But here it's the opposite: The untransformed vector (p, q, 1) is multiplied from the left to the transformation matrix to get the transformed vector.
[01:32:32 CEST] <mkver> That explains the difference between this situation and how this stuff is usually presented in linear algebra books.
[01:34:16 CEST] <jkqxz> Right, yes.  It's flipping the matrix, rather than adding the transformation you would want to apply the flip to another matrix by multiplying.
[01:34:40 CEST] <mkver> Not only you made that error; this error happens every time the SEI is parsed in the codebase.
[01:35:33 CEST] <jkqxz> av_display_rotation_get() and av_display_rotation_flip() are not really doing similar things at all.
[01:35:41 CEST] <jkqxz> *matrix_flip
[01:37:35 CEST] <mkver> av_display_rotation_flip yields the matrix that corresponds to the transformation of applying the given flips after having applied the given matrix.
[01:38:37 CEST] <jkqxz> Yes, this has been present since it was added in 2014.
[01:39:40 CEST] <mkver> av_display_rotation_get meanwhile gives you an angle; the value is only correct for rotations, of course.
[01:40:31 CEST] <jkqxz> So what is the nicest answer to this?  Make a av_display_matrix_rotate() function, and use identity -> matrix_flip -> matrix_rotate in the decoders?
[01:41:01 CEST] <mkver> I thought about creating a new function that applies flips, but before applying the given matrix.
[01:42:07 CEST] <mkver> In other words, where av_display_matrix_flip multiplies columns by +1 or -1, this new function would multiply rows by +1 or -1 (corresponds to a multiplication with a diagonal matrix from the left).
[01:48:38 CEST] <mkver> To another topic: In 4d56f7ab8f627aa140c1ede1bb61305f01cefcdd you made BSF see EOF so that they can flush any delayed packets.
[01:49:06 CEST] <mkver> Does any of the current bsf actually need this? And more importantly: Does this really work? Because it seems that EOF does not even reach process_input_packet. Maybe it did work for avconv and not ffmpeg?
[01:49:47 CEST] <mkver> I created a patch for this. See https://github.com/mkver/FFmpeg/commit/1c3703ad9df38a84128df9db138ff9daf4778922
[01:50:48 CEST] <mkver> But ffmpeg.c is not my forte.
[01:52:07 CEST] <mkver> In particular, there is another if (ist->decoding_needed) { at https://github.com/mkver/FFmpeg/blob/1c3703ad9df38a84128df9db138ff9daf4778922/fftools/ffmpeg.c#L4281 and I don't know what to do at this point. Simply removing said check would give lots of error messages (that one sent another packet to a bsf after eof) when one uses the loop option.
[01:57:58 CEST] <jkqxz> I think vp9_raw_reorder is the only bsf which uses flushing delayed packets.
[02:01:11 CEST] <jkqxz> And it does still work in ffmpeg.
[02:02:07 CEST] <jkqxz> Ah, but it isn't really for streamcopy which is what this is doing.
[02:05:41 CEST] <mkver> So what was the actual reason for 4d56f7ab8f627aa140c1ede1bb61305f01cefcdd if it's not needed? Just because it is cleaner that way?
[02:07:56 CEST] <jkqxz> And indeed it doesn't work in streamcopy.  It doesn't even feed all the input into the bsf.
[02:08:15 CEST] <mkver> Does it work with my patch?
[02:09:14 CEST] <jkqxz> Wrt 4d56f7ab8, I don't know.  Maybe the libav mailing list would say more.
[02:10:33 CEST] <jkqxz> After your patch it does flush but didn't pass all the input?
[02:11:43 CEST] <mkver> Ok. How can I reproduce such a "didn't pass all the input" scenario?
[02:14:17 CEST] <jkqxz> Have a test file <http://ixia.jkqxz.net/~mrt/1ca3b2e86c4d1328a36899910f2fe663/vp9rr.ivf>.  It contains three frames I, P, B.  vp9_raw_reorder should turn them into I, superframe(P, B), show_frame(B).
[02:15:01 CEST] <jkqxz> And it does that correctly when it's used as the output of an encoder, but in streamcopy it creates nothing without your patch or only the I frame with your patch.
[02:17:04 CEST] <mkver> Ok. Maybe I have time to investigate this. (But feel free to solve it yourself, of course.)
[02:19:26 CEST] <mkver> It at least works half-way for what I wanted to have: The information when there are no more frames coming to a bsf (in my case a modified h264_metadata).
[02:20:28 CEST] <jkqxz> What are you doing with it?
[02:21:30 CEST] <mkver> This way I can find out whether a parameter set was unused (i.e. not referenced by any slice) at all and then I can send a side-data-only packet with AV_PKT_DATA_NEW_EXTRADATA side-data that only contains the extradata that is really needed.
[02:21:58 CEST] <mkver> (Of course, support for this had to be added to the relevant muxer (Matroska in my case).)
[02:23:11 CEST] <jkqxz> I think the patch is correct, modulo weird interactions because ffmpeg.c is too complicated.
[02:23:51 CEST] <mkver> Yeah, as I said, ffmpeg.c is not my forte. I don't feel "at home" there.
[02:24:03 CEST] <mkver> At least it passes fate.
[02:24:18 CEST] <jkqxz> I don't think anyone does, which is a problem.
[02:24:29 CEST] <mkver> And I'm still unsure what to do in the looping case.
[02:24:49 CEST] <mkver> What? I thought at least Michael Niedermayer does.
[02:28:09 CEST] <mkver> The patches I mentioned are here: https://github.com/mkver/FFmpeg/commits/h264_metadata, if you want to take a look.
[02:29:23 CEST] <ringo_> Hello
[02:29:31 CEST] <ringo_> I think I posted my question to the wrong channel
[02:29:38 CEST] <ringo_> ffmpeg user instead of ffmpeg devel
[02:30:19 CEST] <ringo_> I want to use nvidia cuvid to decode a file and play over decklink output
[02:30:22 CEST] <TheAMM> No, it was the right place
[02:30:34 CEST] <TheAMM> It was about using ffmpeg, no?
[02:30:53 CEST] <ringo_> no, it was about modifying ffmpeg code to possible fix a bug or limiation
[02:30:58 CEST] <ringo_> no, it was about modifying ffmpeg code to possible fix a bug or limitation
[02:31:44 CEST] <ringo_> I am a contributor to several open source projects including VLC .... and indirectly to ffmpeg as well
[02:32:59 CEST] <ringo_> I want to know if the best way to fix the issue I encountered is to modify the decklink output module to support more pixel formats and do the conversion there
[02:42:28 CEST] <jamrial> jkqxz: that sounds more like work for the vp9_superframe filter
[02:42:45 CEST] <jamrial> the sample you linked, i mean
[02:42:59 CEST] <jamrial> or does vp9_raw_reorder do the same thing?
[02:59:31 CEST] <mkver> I actually don't know what the difference of vp9_raw_reorder and vp9_superframe is supposed to be. But it will have to wait until tomorrow: Good night.
[11:43:25 CEST] <cone-865> ffmpeg 03Matthieu Bouron 07master:d83985ce11d0: avcodec/mediacodecdec: try to receive a frame after signaling EOF to the codec
[11:43:26 CEST] <cone-865> ffmpeg 03Matthieu Bouron 07master:fef5ba40231d: avcodec/mediacodecdec: remove unneeded else block in ff_mediacodec_dec_send()
[11:43:27 CEST] <cone-865> ffmpeg 03Matthieu Bouron 07master:7c2c5c4940a6: avcodec/mediacodecdec: re-indent after previous commit
[13:43:54 CEST] <mkver> jkqxz: My preliminary analysis of vp9_raw_reorder is this: The check at https://github.com/FFmpeg/FFmpeg/blob/master/fftools/ffmpeg.c#L2008 fails for the second and third packet (which therefore aren't even sent to the bsf); if one adds -copyinkf to the command line, the packets are sent, processed and muxed correctly, but without the show-existing-frame at the end. With my patch and -copyinkf, all the packets are output 
[13:43:54 CEST] <mkver> correctly.
[13:45:36 CEST] <mkver> There is just one thing that doesn't look right: The dts of the output packets. They are not monotonous; and they don't indicate the reordering: The dts delta for the superframe (i.e. two frames to decode) ends up being exactly the same like the dts delta for the first frame.
[13:46:24 CEST] <nevcairiel> wasnt raw_reorder some rather unique special case for the vaapi encoder or something because it lacks any sort of proper bitstream generation logic
[13:49:51 CEST] <jkqxz> Why did copyinkf help?  The first packet should already be a key frame.
[13:51:46 CEST] <jkqxz> Ok, that does explain why it was mmissing some of the packets even with your patch.  Given that, it sounds like your patch is a good idea.
[13:51:50 CEST] <mkver> Yes. And because it is a keyframe it gets sent to the bsf. But the next frames don't, because the output stream's frame counter is still zero (after all, the keyframe is cached and not output yet) and because they are not keyframes.
[13:52:18 CEST] <jkqxz> nevcairiel:  Yes.  But it's also the only current bsf with weird delay logic.
[13:52:41 CEST] <mkver> I am actually wondering whether it needs to have this weird delay logic at all.
[13:53:27 CEST] <mkver> The only good reason would be to modify the dts and/or pts of the output packets before outputting them, so that e.g. dts is monotonous.
[13:54:43 CEST] <mkver> If it is just some flags that need to be known later, then they can be kept, but do we really need to keep the actual packets for this? I am not sure about this.
[13:56:59 CEST] <mkver> And even if this delay stuff can't be eliminated completely, I don't get why the keyframe can't be output right away.
[13:57:15 CEST] <jkqxz> I think it should drop the dts stuff entirely and output dts == pts on every frame - I noticed going wrong with larger pyramids in vp9_vaapi but I haven't pursued it yet.
[14:00:22 CEST] <mkver> It already outputs pts == dts (at least in your sample). And that leads to the usual "Application provided invalid, non monotonically increasing dts to muxer" errors.
[14:00:55 CEST] <jkqxz> Because it hasn't been superframed?
[14:01:47 CEST] <mkver> Adding the superframe bsf fixes these errors, that's true. If you are fine with this, then so am I.
[14:04:10 CEST] <jkqxz> The superframe components correctly end up with the same timestamps.  Since it's adding the show-frame packets, there is no way to avoid some of the pre-superframe stream having the same timestamps for some packets.
[14:07:41 CEST] <mkver> Really? What about the following for our sample: I frame: dts -1, pts 0; P frame: dts 0, pts: NOPTS, B frame: dts:1, pts 1, show_existing frame: dts 2, pts 2.
[14:08:31 CEST] <mkver> One could of course use the pts 2 for the P frame, too. At least the dts would be correct then.
[14:14:18 CEST] <jkqxz> The calculation on timestamps to decide how far back to go needs to examine the whole stream before starting.  E.g. a pyramid { I, B1, B2, B3, P } (vp9_vaapi with -bf 3 -b_depth 2) needs -2 dts to make { I, P, B2, B1, show-B2, B3, show-P }, and that sequence could be concatenated after the previous one which only had an offset of -1.
[14:16:48 CEST] <mkver> So because it's impossible, we don't even try and leave it to vp9_superframe. Fine by me.
[14:17:20 CEST] <jkqxz> Yes, exactly :)
[15:09:50 CEST] <mkver> jkqxz: If I am not mistaken, VP9 allows to have coded frames that are never displayed, but just used internally as references in the decoding process. This makes me wonder how uniquely determined the output with show_existing_frames is when one is given the stream without show_existing_frames as input.
[15:12:02 CEST] <mkver> More concretely: If I have I0, P2, B1 and ND (ND = No-Display) in this order as the beginning of the input, then where is the place for the show_existing_frame corresponding to the P frame? Before or after the ND?
[15:22:13 CEST] <nevcairiel> it ultimately doesn't matter where it is
[15:22:41 CEST] <nevcairiel> unless the ND is supposed to replace the reference slot that P went into
[19:44:34 CEST] <llogan> twitter user wants to know which encoders support both intra and non-intra. h26*, libx26*, ffv1, mpeg*, libvpx*, wmv, snow. Any others?
[19:45:38 CEST] <gnafu> libaom* is another prominent up-and-comer.
[19:46:10 CEST] <llogan> forgot about that one
[23:13:42 CEST] <cone-974> ffmpeg 03Paul B Mahol 07master:6473a5d35c16: avformat/id3v2enc: fix bug, CTOC flags take only one byte
[00:00:00 CEST] --- Fri Jun 14 2019