search

cisco / openh264

Open Source H.264 Codec
BSD 2-Clause "Simplified" License
5.48k stars 1.77k forks source link

Reintroduced Frame-Order bug in master/v2.2.0 #3476

Closed akallabeth closed 2 years ago

akallabeth commented 2 years ago

Bug reported in https://github.com/cisco/openh264/issues/3305 has been reintroduced in master and https://github.com/cisco/openh264/tree/v2.2.0

Offending commit: dc17ceea7aae76131e30d72c69f801185e917b05

Easy way to test:

  1. Compile FreeRDP from https://github.com/FreeRDP/FreeRDP/tree/master with cmake -GNinja -DWITH_OPENH264=ON
  2. Connect to some windows RDP server with xfreerdp /v:someserver (AVC mode enabled)
  3. Create a selection rectangle with a mouse left click
  4. Release the rectangle
  5. See that the rectangle is (in part) still visible

To verify the stream can be decoded properly:

  1. Compile FreeRDP from https://github.com/FreeRDP/FreeRDP/tree/master with cmake -GNinja -DWITH_FFMPEG=ON
  2. Retry above
joakim-tjernlund commented 2 years ago

@xiaotiansf , any idea?

xiaotiansf commented 2 years ago

@akallabeth I can take a look at the issue. Could you provide me a sample avc stream recorded from your RDP server?

akallabeth commented 2 years ago

Attaching a raw data dump (just before calling DecodeFrame2) with fwrite(pSrcData, 1, SrcSize, fp); If you need some other format, please ping me.

avc444.dump.raw.zip

xiaotiansf commented 2 years ago

Thanks!

akallabeth commented 2 years ago

@xiaotiansf just a note, as I´ve played with the raw data a bit: The problem is not always visible if played as RAW video as the frame decoding works, but the frames are not always emitted in time, e.g. after DecodeFrame2

xiaotiansf commented 2 years ago

@akallabeth Thanks for the note!

xiaotiansf commented 2 years ago

@akallabeth I have identified the issue. Somehow, POC (Picture Order Count) for every decoded picture is 0 and it does not increase (it is supposed to increase in a GOP). ReorderPicturesInDisplay can't handle reordering correctly. I'll try to fix it as soon as I can.

xiaotiansf commented 2 years ago

@akallabeth Well after I have dug deep into the issue using openh264 console app "h264dec" and actually did not really find any thing wrong. I always got correct decoded yuv video without any artifacts (which I view on Yuvview app) from your avc444.dump.raw.zip.

I am guessing the artifacts might be caused by the app tries to copy specific decoded image from decode buffers while decoder buffers are being updated for next decoded buffer before copying is completed. I am wondering if I have to build https://github.com/FreeRDP/FreeRDP/tree/master to reproduce the issue?

As for Big_Buck_Bunny_1080_10s_30MB.mp4, that seems to be a very different issue as openh264 somehow found video gap errors for the stream. This is also an issue. but it should be put Big_Buck_Bunny_1080_10s_30MB.mp4 into separate issue.

akallabeth commented 2 years ago

@xiaotiansf sorry for the delay, did miss your comment. Yes, it might have to do with our use case. We need to copy the data of the decoded frame right after DecodeFrame2 for post processing. If the frame data of the relevant area is not available invalid/old image data will be copied and artifacts will start showing.

Testing with FreeRDP from master (or any other version since 2.0 actually) will certainly help check if the issue is there. (if you test on debian/ubuntu based systems see https://github.com/FreeRDP/FreeRDP/wiki/Compilation#nightly-build-for-debian-based-systems - you only need the 3 lines to build a nightly package. to enable OpenH264 you need to have it installed and add DEB_CMAKE_EXTRA_FLAGS := -DWITH_OPENH264=ON \ in packaging/deb/freerdp-nightly/rules )

xiaotiansf commented 2 years ago

@akallabeth I'll try it on debian/ubuntu with FreeRDP to see if I can reproduce the issue.

xiaotiansf commented 2 years ago

@akallabeth I follows: build and install openh264 from master sudo apt build-dep freerdp2-x11 ln -s packaging/deb/freerdp-nightly debian dpkg-buildpackage and then update DEB_CMAKE_EXTRA_FLAGS := -DWITH_OPENH264=ON \ in packaging/deb/freerdp-nightly/rules But after build, when I check xfreerdp /buildconfig, it still shows WITH_OPENH264=OFF. Then launch xfreerdp server, check the remote desktop, it looks everything okay. It could be that that I am not using openh264

akallabeth commented 2 years ago

@xiaotiansf ok, then here are some exact steps with which I can create a working test binary (assuming you already have installed the build dependencies with sudo apt build-dep freerdp2-x11:

cd openh264
meson setup -Dprefix=/tmp/xxx xxx
ninja -C xxx install
cd ..
mkdir -p freerdp-build
cd freerdp-build
cmake -GNinja -DCMAKE_INSTALL_PREFIX=/tmp/xxx -DWITH_OPENH264=ON -DWITH_FFMPEG=OFF ../freerdp
cmake --build . --target install
/tmp/xxx/bin/xfreerdp /v:server /u:user /p:pwd /gfx:avc444 /log-filters:com.freerdp.channels.rdpgfx.client:trace

The /log-filters:com.freerdp.channels.rdpgfx.client:trace is there to check your target server is really using a AVC mode. If lines similar to [09:42:25:996] [128532:128551] [DEBUG][com.freerdp.channels.rdpgfx.client] - H264_METABLOCK: numRegionRects: 1 are printed, the server is using the correct mode, if not you can´t test the bug.

xiaotiansf commented 2 years ago

@akallabeth I got the following errors from the command "cmake --build . --target install" (got the same error with sudo cmake...)

[1/189] Creating library symlink winpr/libwinpr/libwinpr3.so.3 winpr/libwinpr/libwinpr3.so FAILED: winpr/libwinpr/libwinpr3.so.3 winpr/libwinpr/libwinpr3.so /usr/bin/cmake -E cmake_symlink_library winpr/libwinpr/libwinpr3.so.3.0.0 winpr/libwinpr/libwinpr3.so.3 winpr/libwinpr/libwinpr3.so && : CMake Error: failed to create symbolic link 'winpr/libwinpr/libwinpr3.so.3': operation not permitted CMake Error: cmake_symlink_library: System Error: Operation not permitted CMake Error: failed to create symbolic link 'winpr/libwinpr/libwinpr3.so': operation not permitted CMake Error: cmake_symlink_library: System Error: Operation not permitted

Note that I only see "libwinpr3.so.3.0.0" exists in winpr/libwinpr.

akallabeth commented 2 years ago

Permission problems caused by running with sudo, retry with a fresh checkout

akallabeth commented 2 years ago

Don't run anything here with sudo, /tmp is writable as user so no need for that

xiaotiansf commented 2 years ago

I retried with the fresh checkout and got the same error without sudo. In fact, I did not sudo in first try.

akallabeth commented 2 years ago

@xiaotiansf and you ran it exactly as above?

I´ve attached a little script which builds fine on all my test machines:

  1. It downloads openh264 and freerdp to a temporary folder
  2. It builds openh264, installs to /tmp/xxx
  3. It builds freerdp, installs to /tmp/xxx

The script is #!/bin/bash -xe, so it aborts whenever something fails. (e.g. some dependency / tool missing)

build.sh.zip

xiaotiansf commented 2 years ago

With your build.sh, it builds successfully. Thanks! But when I launch the command /tmp/xxx/bin/xfreerdp /v:server /u:user /p:pwd /gfx:avc444 /log-filters:com.freerdp.channels.rdpgfx.client:trace

and gets the following error:

[16:02:49:961] [20869:20870] [INFO][com.freerdp.core] - freerdp_connect:freerdp_set_last_error_ex resetting error state [16:02:49:961] [20869:20870] [INFO][com.freerdp.client.common.cmdline] - loading channelEx rdpdr [16:02:49:961] [20869:20870] [INFO][com.freerdp.client.common.cmdline] - loading channelEx rdpsnd [16:02:49:961] [20869:20870] [INFO][com.freerdp.client.common.cmdline] - loading channelEx cliprdr [16:02:49:961] [20869:20870] [INFO][com.freerdp.client.common.cmdline] - loading channelEx drdynvc [16:02:49:274] [20869:20870] [INFO][com.freerdp.primitives] - primitives autodetect, using optimized [16:02:49:279] [20869:20870] [ERROR][com.freerdp.core] - freerdp_tcp_is_hostname_resolvable:freerdp_set_last_error_ex ERRCONNECT_DNS_NAME_NOT_FOUND [0x00020005]

akallabeth commented 2 years ago

@xiaotiansf well, the /tmp/xxx/bin/xfreerdp /v:server /u:user /p:pwd /gfx:avc444 /log-filters:com.freerdp.channels.rdpgfx.client:trace is an example, you need to connect to an actual RDP server. (e.g. any windows 10, users might need to be added to remote access group)

xiaotiansf commented 2 years ago

I did launch ( xfreerdp client) /tmp/xxx/bin/xfreerdp /u:user: /p:pwd /v:127.0.0.1:3389 /gfx:avc444 /log-filters:com.freerdp.channels.rdpgfx.client:trace, where xrdp server is at localhost:3389. I see a window (black) pop-up with title "FreeRDP: 127.0.0.1" but nothing else. Do I have to launch xrdp on a separate machine?

akallabeth commented 2 years ago

@xiaotiansf xrdp does not support the required modes, please connect to a windows machine.

xiaotiansf commented 2 years ago

I see. Thanks.

xiaotiansf commented 2 years ago

@akallabeth I gets the following "Certificate verification failure" error when I try to remote to my windows 10 pro machine

/tmp/xxx/bin/xfreerdp /u:user /p:pwd /v:192.168.1.15 /gfx:avc444 /log-filters:com.freerdp.channels.rdpgfx.client:trace [21:28:41:261] [12943:12944] [INFO][com.freerdp.core] - freerdp_connect:freerdp_set_last_error_ex resetting error state [21:28:41:261] [12943:12944] [INFO][com.freerdp.client.common.cmdline] - loading channelEx rdpdr [21:28:41:261] [12943:12944] [INFO][com.freerdp.client.common.cmdline] - loading channelEx rdpsnd [21:28:41:261] [12943:12944] [INFO][com.freerdp.client.common.cmdline] - loading channelEx cliprdr [21:28:41:261] [12943:12944] [INFO][com.freerdp.client.common.cmdline] - loading channelEx drdynvc [21:28:41:576] [12943:12944] [INFO][com.freerdp.primitives] - primitives autodetect, using optimized [21:28:41:578] [12943:12944] [INFO][com.freerdp.core] - freerdp_tcp_is_hostname_resolvable:freerdp_set_last_error_ex resetting error state [21:28:41:578] [12943:12944] [INFO][com.freerdp.core] - freerdp_tcp_default_connect:freerdp_set_last_error_ex resetting error state [21:28:41:648] [12943:12944] [INFO][com.freerdp.core.nego] - RDP_NEG_RSP::flags = { [0x1f] EXTENDED_CLIENT_DATA_SUPPORTED|DYNVC_GFX_PROTOCOL_SUPPORTED|RDP_NEGRSP_RESERVED|RESTRICTED_ADMIN_MODE_SUPPORTED|REDIRECTED_AUTHENTICATION_MODE_SUPPORTED } [21:28:41:664] [12943:12944] [WARN][com.freerdp.crypto] - Certificate verification failure 'unable to get local issuer certificate (20)' at stack position 0 [21:28:41:664] [12943:12944] [WARN][com.freerdp.crypto] - CN = MyVideoDesktop [21:28:41:665] [12943:12944] [INFO][com.freerdp.crypto] - known_hosts=1 [21:28:42:770] [12943:12944] [INFO][com.winpr.timezone] - winpr_get_unix_timezone_identifier_from_file: tzid: America/Los_Angeles [21:28:42:770] [12943:12944] [INFO][com.winpr.timezone] - winpr_get_timezone_from_link: tzid: America/Los_Angeles [21:28:42:770] [12943:12944] [INFO][com.winpr.timezone] - tzid: America/Los_Angeles [21:28:42:071] [12943:12944] [INFO][com.freerdp.gdi] - Local framebuffer format PIXEL_FORMAT_BGRX32 [21:28:42:071] [12943:12944] [INFO][com.freerdp.gdi] - Remote framebuffer format PIXEL_FORMAT_BGRA32 [21:28:42:073] [12943:12944] [INFO][com.freerdp.client.x11] - Virtual core XTEST pointer button device (id: 4, mode: 10) [21:28:42:073] [12943:12944] [INFO][com.freerdp.client.x11] - Logitech MK700 button device (id: 13, mode: 7) [21:28:42:073] [12943:12944] [INFO][com.freerdp.client.x11] - Logitech M510 button device (id: 14, mode: 20) [21:28:42:073] [12943:12944] [INFO][com.freerdp.client.x11] - Logitech K850 button device (id: 15, mode: 7) [21:28:42:073] [12943:12944] [INFO][com.freerdp.client.x11] - Logitech M585/M590 button device (id: 16, mode: 20) [21:28:42:073] [12943:12944] [INFO][com.freerdp.client.x11] - SteelSeries SteelSeries KLC button device (id: 17, mode: 7) [21:28:42:073] [12943:12944] [INFO][com.freerdp.client.x11] - CUST0001:00 06CB:CDAD Mouse button device (id: 18, mode: 7) [21:28:42:073] [12943:12944] [INFO][com.freerdp.client.x11] - CUST0001:00 06CB:CDAD Touchpad button device (id: 19, mode: 7) [21:28:42:074] [12943:12944] [INFO][com.freerdp.client.x11] - PS/2 Generic Mouse button device (id: 21, mode: 7) X Error of failed request: XI_BadDevice (invalid Device parameter) Major opcode of failed request: 131 (XInputExtension) Minor opcode of failed request: 46 () Device id in failed request: 0x4800001 Serial number of failed request: 69 Current serial number in output stream: 69

akallabeth commented 2 years ago

@xiaotiansf sorry, but I suspect your installation is broken, or do you really have 5 mice attached to your system? I´ve just setup a ubuntu 18.04 VM (usually use 20.04 as 18.04 is ancient) and the whole instructions work really well. Just use a VM too, debugging exotic device problems is too much just to reproduce this frame issue.

xiaotiansf commented 2 years ago

@akallabeth After I create and install my SSL certificate, I am able to remote to my windows desktop. But I notice that not all contents of my remote desktop is being refreshing when I click mouse on it or open a new browser window. My question is how I capture the encoded h264 stream so that I can compare encoded stream to decoded stream through stream viewer?

xiaotiansf commented 2 years ago

@akallabeth Well, for test and comparison purpose, I created a branch which completely disables the decoded frame buffering. I attached the script build_disable_openh264_buffering.zip and I don't really see that it behaviors differently between disabled/enabled frame buffering. Could you use my updated script to verify? Thanks.

akallabeth commented 2 years ago

@xiaotiansf The disabled decoded frame buffering branch works for me here.

I´ve created branch https://github.com/akallabeth/FreeRDP/tree/h264_dump which dumps:

  1. Raw input data to /tmp/freerdp-raw.dump
  2. For each decoded frame the yuv buffers to /tmp/freerdp_y_frame_00000000.raw, /tmp/freerdp_u_frame_00000000.raw, /tmp/freerdp_v_frame_00000000.raw
xiaotiansf commented 2 years ago

@akallabeth I see. I confirmed that openh264 console h264dec generates the exact same yuv output files with/without frame buffering. This does indicate that the way that xfreerdp fetches a decoded frame is not compatible when openh264 uses frame buffering. My suggestion for the fix for xfreerdp is to use h264 baseline (profile idc = 66) instead of main (profile idc =77). My understanding is that for remoting desktop, it does not need B-frame. Baseline does not support B-frame so it never needs to buffer a frame.

akallabeth commented 2 years ago

@xiaotiansf I´ll try that option. As for the bug, I agree that the files will have the correct result eventually, but the frames (when buffering is in use) will be emitted too late. We do the following:

  1. Decode H264 raw data
  2. (optionally) decode a second H264 frame (different reference frame, contains data to upgrade YUV4:2:2 to YUV 4:4:4 in proprietary format)
  3. copy the changed rectangles to YUV 4:4:4 internal buffer
  4. Convert YUV 4:4:4 to RGB data
akallabeth commented 2 years ago

@xiaotiansf ok, I´ve found the issue. The plugin was written a long time ago and newer openh264 versions introduced FlushFrame. Calling this before copying the frame data indeed fixes the graphical artifacts.

akallabeth commented 2 years ago

So, closing as not a bug but API change