[discrepancy with libav] avcodec_encode_video2 merges packet side data with the packet body

[Lastique]

We have an application which uses ffmpeg to encode video in h263. The application requests h263_mb_info side data by setting the "mb_info" option on the encoder context:

av_opt_set_int(av_ctx, "mb_info", 1400, AV_OPT_SEARCH_CHILDREN)

Later we call avcodec_encode_video2() to encode frames. The problem is that ffmpeg avcodec_encode_video2() returns AVPackets with the encoded frame and some amount of junk at the end. From the source code I can see that the junk is actually the encoded side data (mb_info) appended to the h263 packet by av_packet_merge_side_data() which is called by avcodec_encode_video2().

This appended side data prevents the packets from being decoded by ffmpeg 2.2.1 (I tried both decoding through the API and ffplay). Decoding results in many messages such as:

slice end not reached but screenspace end (399 left 019CE3, score= -7)
concealing 88 DC, 88 AC, 88 MV errors in P frame

and corrupted image.

This is also different from libav behavior. At least 9.x and 10.x versions don't merge side data into the packet body, and when using libav we don't have the decoding problem.

Looking at the libavcodec code I don't really understand why the call to av_packet_merge_side_data() is needed. Side data, at least mb_info in case of h263, is not the part of the encoded media and thus should not be part of the packet body. Since the packet size (AVPacket::size) includes the appended side data, there is no way to obtain the size of the actual media. I suspect, av_packet_split_side_data() can be used to restore the original un-merged packet, but (a) it is not documented and (b) doing av_packet_merge_side_data() and av_packet_split_side_data() is a waste of memory and CPU (note: we pre-allocate packet data buffer to optimize performance). Therefore my preferred solution would be to remove the call to av_packet_merge_side_data() from avcodec_encode_video2().

If that call cannot be removed then what is the correct way to obtain the actual media from the merged packet?

[Carl Eugen Hoyos]

Is this not reproducible with current FFmpeg git head?

How can I reproduce the problem with ffplay?

[Lastique]

Replying to cehoyos:

Is this not reproducible with current FFmpeg git head?

I did not test git head, but looking at the code of avcodec_encode_video2() in git master it still calls av_packet_merge_side_data() so it must have the same problem.

How can I reproduce the problem with ffplay?

I don't know how to make ffmpeg request mb_info from the command line. I have an output video file we got with our application that shows decoding errors I described. I can upload it tomorrow, if you're interested. By the way, the same file is played without errors by libav's ffplay. I guess, it's more tolerant towards the junk at the end of encoded frames.

[llogan]

Replying to Lastique:

By the way, the same file is played without errors by libav's ffplay

That fork does not currently supply a "ffplay" as far as I know.

[Lastique]

Replying to llogan:

Replying to Lastique:

By the way, the same file is played without errors by libav's ffplay

That fork does not currently supply a "ffplay" as far as I know.

I tried on Kubuntu 13.10, it does have ffplay, which is actually a link to avplay. Doesn't matter.

[gjdfgh]

The side data merging is a really ugly, crappy, disgusting etc. (insert more deprecatory words here), that has to be disabled by each user.

I use this code to disable it:

#if LIBAVFORMAT_VERSION_MICRO >= 100
    /* Keep side data as side data instead of mashing it into the packet
     * stream.
     * Note: Libav doesn't have this horrible insanity. */
    av_opt_set(av_ctx, "fflags", "+keepside", 0);
#endif

But I'd argue that it should be disabled by default. User applications which pass only packet data (without side data), i.e. are broken, could merge the side data manually by calling an API function.

[Lastique]

Unfortunately, that flag only affects libavformat and the issue I'm having is with libavcodec. As far as I can see, there is no way to disable side data merging in libavcodec except by patching and recompiling it.

[Lastique]

A test case illustrating the problem.

[Lastique]

I managed to create a test case that roughly reflects our application behavior and shows the problem.

1. Compile and run without arguments, the test will produce output.avi. This file contains encoded video frames with junk at the end. This file does not play correctly with ffmpeg's ffplay and plays fine by avplay from Kubuntu 13.10 (version 0.8.10-6:0.8.10-0ubuntu0.13.10.1).
2. Rename output.avi to output_bad.avi for future reference.
3. Run the test again with --without_side_data command line argument. It will produce the correct output.avi, which can be played by both ffplay and avplay.
4. During both runs the test program outputs encoded frame size, you can notice that the first run produces larger frames. output_bad.avi will also be larger than the correct output.avi.

I have another note to add. av_packet_merge_side_data() reallocates the packet buffer and puts encoded data and side data in the new buffer. It frees the old buffer and sets AVPacket::data to the new buffer. This breaks the interface of avcodec_encode_video2(), which is documented to use the user's buffer if provided. We rely on that contract in our code, that's why we're basically using the freed memory with junk at the end. This is reflected in the test case.

After this discovery I have to agree with gjdfgh (comment:5). Please, remove side data merging.

[gjdfgh]

Note that you can also convert between merged and separate side data with av_packet_merge_side_data() and av_packet_split_side_data().

Does calling av_packet_split_side_data() on the packet before writing it to the muxer help? (It obviously should, but I'm not sure when exactly libavcodec "merges" the side data. Edit: oh, it's simply called at the end of the avcodec_encode_video2 implementation. Edit2: and I should learn to read!)

[Lastique]

The patch removes side data merging.

[Michael Niedermayer]

That patch breaks ABI and is removing quite a bit of unrelated code.

[Lastique]

I'm not insisting on merging it, that's the patch we used to solve the problem on our side.

I don't see where ABI is broken. No public structures or function signatures are changed. The patch changes the behavior to be in sync with libav, and that was the intention.

Could you clarify what unrelated code is removed?

[mkver]

Fixed in 809780ca425afbb1892cbd6957ca2f1cd7d64396 (additionally the whole side-data-splitting/merging stuff has been deprecated in d682ae70b4b3a53fb73ec30281f9f4cfbc531edd and its remnants will be removed very soon).