search

dholroyd / h264-reader

Rust reader for H264 bitsream syntax
Apache License 2.0
70 stars 25 forks source link

convenience methods? #9

Open scottlamb opened 4 years ago

scottlamb commented 4 years ago

What are your feelings on additional convenience methods / methods that interpret the raw fields? I find the H.264 spec rather dense and want to get a few "simple" things without studying it in detail. I imagine other folks will be in the same boat.

Here are some I'd like to have and why I think it's better for the crate to compute them than for callers to do so (badly):

dholroyd commented 4 years ago

Sounds great!

Think I can make some quick additions for the first two; for the second two I'd be happy to add helpers, but can't commit to researching the proper implementations right now.

dholroyd commented 4 years ago

I hope 44ebdffdb82b029b6f718533face733e79ac8586 addresses the second item in the list (but feel free to propose adjustments).

scottlamb commented 4 years ago

Thank you!

For rfc6381, it would be great if there was a separate crate for this; I have other places I could use this. Was thinking something providing an enum Codec { Avc1 { ... }, etc } with appropriate impl ToString/TryFrom. If you want to see that, I can set something up.

That'd be awesome, thank you!

(On the subject of other codecs and crates: do you have any plans for H.265? no pressure.)

I hope 44ebdff addresses the second item in the list (but feel free to propose adjustments).

Thanks!

I'll start looking into the second two then.

I'm already having second thoughts about bitrate. It looks like unfortunately even defining an interface for it is a bit of a mess because there's not just one result:

First, there's multiple CPBs. I'm not quite sure why yet; maybe because of scalable video coding.

Then there's VUI vs NAL. I haven't found an explanation in the spec itself yet, but I found this snippet on someone's website:

The difference between NAL HRD and VCL HRD is whether SPS/PPS/SEIs are counted or not in HRD calculations. In NAL HRD mode all non-VLC NALs (i.e. SPS, PPS, SEIs) as well as VCL NALs (i.e. Slices) are counted in HRD while in VCL HRD mode non-VCL NALs are ignored (consequently in VCL HRD mode access unit may be smaller than that in NAL HRD).

In my opinion the reason for VCL HRD mode: in some applications SPS/PPS are given a priory or signaled out-of-band, therefore there is no sense to include these NALs in HRD process.

Of course it's optional to specify HRD parameters. There's also a max allowable one defined by the level and profile. One of the CPBs has to be within that max (see A.3.1.i).

dholroyd commented 4 years ago

Here's the initial sketch of codec support.

My thinking is that with a dependency on this new crate, we can then extend the Sps type like,

    fn rfc6381(&self) -> rfc6381_codec::Codec {
        rfc6381_codec::Codec::avc1(self.profile_idc.0, self.constraint_flags.0, self.level_idc)
    }

so to get the string you want it should be sps.rfc6381().to_string() or format("{}", sps.rfc6381())

dholroyd commented 4 years ago

do you have any plans for H.265?

I understand that a lot of the infrastructure like NALU syntax is the same, so yeah I'd like to make it easy to either support H.265 in this crate, maybe renamed (h26x-reader), or separate the generic stuff into third crate (h26x-reader + h264-reader + h265-reader).

I've not really got any h265 material passing through my hands at the moment to motivate work on that, but hopefully that'll change at some point!

I raised #10 to track that thought.

dholroyd commented 4 years ago

I hope 44ebdff addresses the second item in the list (but feel free to propose adjustments).

I notice the Chromium codebase takes care to avoid integer overflow in the equivalent spot. Sorry for the churn, but pixel_dimensions() should be changed to return Result.

scottlamb commented 3 years ago

I'm making progress on my pure-Rust RTSP client so taking a fresh look at this stuff.

dholroyd commented 3 years ago

Regarding rfc6381_codec, you've reminded me of a pending change that I've just committed, adding dependencies on mpeg4-audio-const and mp4ra-rust crates, which define the constants which rfc6381_codec had just defined locally before. Hopefully this does not seem like over-engineering things! :sweat_smile:

dholroyd commented 3 years ago

It might also be nice to have a convenience method to construct an AVCDecoderConfigurationRecord

Any existing crate/type you had in mind? I assume it doesn't make sense to define a type local to h264-reader since it wouldn't interoperate with any existing MP4 writer.

scottlamb commented 3 years ago

I'm not aware of anything. I was thinking of just being able to get the raw bytes as a Vec<u8>, but yeah in my dream world there'd be a broadly used non-codec-specific, non-application-specific video and audio parameters crate. My high-level RTSP crate isn't the best place for it, but I'm working toward making something like there which can produce:

This is the kind of stuff that I'd imagined the rust-av project doing but I'm not sure if it's gone anywhere. [edit: I probably should check in with them, but I just want to get something working right now.] Seems like the webrtc-rs stuff is also in adjacent space (they have their own RTP and RTCP crates now also).

btw, I'm also working on stuff for (de)muxing codecs from RTP packets to access units, which in theory might make more sense in your rtp-rs crate, with the caveats that (a) my code right now assumes no dropped RTP packets which probably will work for interleaved RTSP but not whatever other ways folks use RTP, and (b) I'm working with Bytes at the moment which may be more "opinionated" than you want.

fwiw, my (very much a work in progress) crate produces the AVCDecoderConfiguration here, although my code is sketchy (only handles a single SPS and PPS).