ralfbiedert / openh264-rs

Idiomatic Rust wrappers around OpenH264.
66 stars 32 forks source link

Invalid access decoding packet stream #7

Closed BlackHoleFox closed 2 years ago

BlackHoleFox commented 2 years ago

Hiya. Thanks for making this crate, its looking like a much better option then dragging in all of ffmpeg by a mile :)

While trying to decode an H264 video stream coming from an Android phone, I've kept running into issues with the library making bad accesses and then erroring out. While I'm not sure how much of it is my fault, I haven't been able to figure out what, if anything, I'm doing wrong that'd lead to issues. fwiw, ffmpeg decodes the frames without issue.

I threw together a minimal reproduction below and included the H264 stream bytes up until the point it fails. Thanks in advance if or when you get a chance to look at it.

Reproduction source:

use openh264::decoder::{Decoder, DecoderConfig};

const VIDEO_STREAM: &[u8] = include_bytes!("../stream_dump.bin");

fn main() {
    println!("attempting decode");
    let config = DecoderConfig::new().debug(true);
    let mut decoder = Decoder::with_config(config).unwrap();

    let _packet = decoder.decode(VIDEO_STREAM).unwrap();
}
[package]
name = "decoding_repro"
version = "0.1.0"
edition = "2018"

[dependencies]
openh264 = "0.2.4"

Packet stream: stream_dump.zip

OpenH264 logs:

[OpenH264] this = 0x000002204D7C6CC0, Info:CWelsDecoder::SetOption for ERROR_CON_IDC = 0.
[OpenH264] this = 0x000002204D7C6CC0, Error:NAL_UNIT_PREFIX: DecInitBits() fail due invalid access.
[OpenH264] this = 0x000002204D7C6CC0, Info:decode failed, failure type:4

[OpenH264] this = 0x000002204D7C6CC0, Info:WelsRequestMem(): memory alloc size = 2960 * 1440, ref list size = 3
[OpenH264] this = 0x000002204D7C6CC0, Info:SyncPictureResolutionExt(), overall memory usage: 52380008 bytes
[OpenH264] this = 0x000002204D7C6CC0, Info:DecodeFrameConstruction(): will output first frame of new sequence, 2960 x 1440, crop_left:0, crop_right:0, crop_top:0, crop_bottom:0, ignored error packet:0.
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { native: 4, decoding_state: 0, misc: None }', src\main.rs:10:48
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[OpenH264] this = 0x000002204D7C6CC0, Info:CWelsDecoder::UninitDecoderCtx(), openh264 codec version = openh264 default: 1.4.
[OpenH264] this = 0x000002204D7C6CC0, Info:CWelsDecoder::UninitDecoder(), verify memory usage (0 bytes) after free..
[OpenH264] this = 0x000002204D7C6CC0, Info:CWelsDecoder::~CWelsDecoder()
ralfbiedert commented 2 years ago

Thanks for the report. To start with the good news, the stream can actually decode, i.e., this works:

#[test]
fn decode() -> Result<(), Error> {
    let src = &include_bytes!("data/stream_dump.bin")[..];

    let packet_lengths = [30, 8, 25656];

    let config = DecoderConfig::default().debug(false);
    let mut decoder = Decoder::with_config(config)?;

    let mut p = 0;

    for l in packet_lengths {
        if l == 8 {
            p += l;
            continue;
        }

        decoder.decode(&src[p..p + l])?;

        p += l;
    }

    Ok(())
}

The issue is the 8 byte packet. Inspecting the file with an H264 analyzer it looks like so:

image

The 8 bytes is what trips OpenH264 up. This is not just an issue of this lib, both the H264 analyzer I used above doesn't quite know what the packet is, as well as h264dec fails this packet with an error, but then continues anyway:

H264 source file name: x.264..
Sequence output file name: 1.yuv..
------------------------------------------------------
(1, 21),
(23, 7),
[OpenH264] this = 0x0x7fffee0d7080, Error:NAL_UNIT_PREFIX: DecInitBits() fail due invalid access.
(31, 7),
(39, 25655),
-------------------------------------------------------
iWidth:         2960
height:         1440
Frames:         1
decode time:    0.049273 sec
FPS:            20.295091 fps
-------------------------------------------------------

I'm think that packet can safely be ignored, and I think this might even be what VLC (or a reasonable decoder) should be doing, and maybe there's even a OpenH264 option for that.

With all that said there are 2 options forward:

I think we should do both. The second option will address your problem and I we should have an API for that in any case. The first one is a bit of an unknown, but it would be good to export more encoder / decoder options anyway.

Unfortunately I won't have much time doing either, but I'd very much welcome PRs.

My preferred solution for the segmentation part would be a separate API call accepting a &[u8] in return emitting an Iterator<&[u8]>, splitting the original stream at NAL boundaries. That call should be non-allocating, and either use some hand-crafted NAL parsing (I think that shouldn't be too hard, but haven't tried) or use OpenH264 for that, which I believe has an API built in (but might do some ugly things like aliasing the original buffer or use an internal allocation).

In any case, once that API exists decoding should look as simple as:

#[test]
fn decode() -> Result<(), Error> {
    let src = &include_bytes!("data/stream_dump.bin")[..];

    let config = DecoderConfig::default().debug(false);
    let mut decoder = Decoder::with_config(config)?;

    for packet in todo_split_nal(src) {
        // If this fails you should continue anyway as some NALs might just be corrupt. 
        let _result = decoder.decode(packet);
    } 

    Ok(())
}
ralfbiedert commented 2 years ago

Turns out that was really just trivially splitting for 001, fixed and pushed in 0.2.7.

BlackHoleFox commented 2 years ago

That was a very quick fix since I didn't even have a chance to open this issue back up yet today :) Thanks a ton for the great explanation into why this occurred. Thats a huge help for me as I'm not anywhere near knowledgeable in A/V work.

For the actual fix, once again thanks. Also sorry for essentially forcing you to write out a clause about decoder errors not being actual, uh, bugs. I'll keep that in mind in the future.