kmsbufferpool: "The buffer will be not be reused. This is most likely a bug in this GstBufferPool subclass"

[joedrago]

Hello! --

I've been playing with gstreamer on an Orange Pi 5 running Armbian Jammy (64bit), and I've been trying to test both arm64 and armhf. I've been using liujianfeng1994's PPA to setup playback (as documented here).

However, for my initial testing on armhf, I was failing to find a recent .deb of gstreamer1.0-rockchip1. I eventually found an old package (gstreamer1.0-rockchip1_1.10-5ayufan12_armhf.deb) from 2017 (!), and found that basic h264 playback was nice and smooth. I eventually wanted to have a more recent version of gstreamer1.0-rockchip1, so I cloned your rockchip_mirrors repo and successfully built the latest version using the attached armhf cross-file. Using the latest code, I ran into logs full of this:

0:00:05.837257189 77407 0xf6502d50 WARN            kmsallocator gstkmsallocator.c:526:gst_kms_allocator_dmabuf_import:<KMSMemory::allocator> Failed to close GEM handle: Invalid argument 22
0:00:05.852249697 77407 0xf6502d50 WARN              bufferpool gstbufferpool.c:1246:default_reset_buffer:<kmsbufferpool2> Buffer 0xf6525150 without the memory tag has maxsize (3110400) that is smaller than the configured buffer pool size (3133440). The buffer will be not be reused. This is most likely a bug in this GstBufferPool subclass
0:00:05.891802045 77407 0xf6502d50 WARN            videodecoder gstvideodecoder.c:3668:gst_video_decoder_clip_and_push_buf:<mppvideodec0> Dropping frame due to QoS. start:0:00:04.766666476 deadline:0:00:04.766666476 earliest_time:0:00:04.820110926

I did a little bit of digging in the gstreamer source to try to make sense of it, and it seems to do with the allocator making buffers that are for 1920x1080, but mppvideodec wanting 1920x1088 instead. I was trying to figure out a smart way to hack/fix it, but the allocator just doesn't have the vstride information, so I figure I'd ask an expert for help! Is the maxsize bug a known issue? Is there a good patch I should be using? I'd appreciate any information you have.

This is an easy repro:

curl -O https://test-videos.co.uk/vids/bigbuckbunny/mp4/h264/1080/Big_Buck_Bunny_1080_10s_5MB.mp4
ffmpeg -i Big_Buck_Bunny_1080_10s_5MB.mp4 -c:v copy -f h264 bunny.es

GST_DEBUG=2 gst-launch-1.0 filesrc location=bunny.es ! h264parse ! mppvideodec ! kmssink plane-id=68

(choose whatever plane-id you want)

armhf.txt

[joedrago]

Okay, I've done some additional bisecting and testing and I think I've discovered the issue. It appears that the implementation of gst_mpp_info_changed() is almost exactly backwards? Looking at this code:

https://github.com/JeffyCN/rockchip_mirrors/blob/cce848d65143ad817f4b4f6cd54e4ed6ac622809/gst/rockchipmpp/gstmpp.c#L439

It is supposed to return whether or not the info changed, but has blocks like this:

  if (GST_VIDEO_INFO_WIDTH (info) != width)
    return FALSE;

  if (GST_VIDEO_INFO_HEIGHT (info) != height)
    return FALSE;

I believe blocks like this would make more sense in a function named gst_mpp_info_matches() (say), but in this case the logic is inverted. If I change this function to return TRUE when I detect a change and FALSE at the end of the function, playback works correctly. I can supply a patch if you agree that this is the issue, but I'm a big confused at this line:

  if (GST_VIDEO_INFO_N_PLANES (info) == 1)
    return TRUE;

This is the only conditional block that returns TRUE in the whole function as it exists right now, and it is looking specifically for a hardcoded count of 1 plane, so I'm not sure if this is the only correct check in the function or if it is also backwards.

Let me know what you think; thanks in advance.

[JeffyCN]

it's an knowing issue, which should be fixed in internal repo.

i've updated this mirror just now.

[JeffyCN]

the one plane check is because we don't need to check other things for single plane format(only need format width height and hor-stride)

[joedrago]

Thanks for the reply; I'll close this issue.