scottlamb / retina

High-level RTSP multimedia streaming library, in Rust
https://crates.io/crates/retina
Apache License 2.0
218 stars 46 forks source link

h.265 support #57

Open Sculas opened 2 years ago

Sculas commented 2 years ago

It would be nice to have support for H.265, since my devices support it. I already read in a different issue that you haven't added support for it yet. You could use this as a tracking issue whenever you decide to add it, if you want to :)

scottlamb commented 2 years ago

Maybe you saw this Moonfire-NVR issue? tl;dr: it's quite hard to support H.265 in Moonfire because the UI is browser-based, and browsers don't have built-in support for H.265 as they do for H.264.

Supporting H.265 in Retina should be a lot easier, but if your goal is to use it with Moonfire, may not be that useful on its own.

nemosupremo commented 1 year ago

@scottlamb I'm looking into adding h265 support into Retina.

I have to first (1) get familiar with RTP then (2) get familiar with how retina is framing h264 data. I think the public API shouldn't need to change (essentially you still get a VideoFrame, and then figuring out if the "frame" is an idr, or dependent frame). Likewise for the VideoParameters struct unless I come across something that is needed for gstreamer to ingest an h265 frame. I think I may be some time away from an initial PR, but just wanted to make sure you didn't have any already existing plans or prototypes.

scottlamb commented 1 year ago

I don't have any plans for H.265. A PR would certainly be welcome. And, yeah, I expect the existing API to be compatible with H.265. (and if not: we can do also make a major version bump as necessary.)

nemosupremo commented 11 months ago

@scottlamb fyi nemosupremo/retina#1

It seems the existing api is compatible, were currently in the process of testing against our cameras for our software but we will be opening a PR here soon.

artem-zinnatullin commented 7 months ago

Hi @nemosupremo, how is HEVC support looking so far? I've seen some progress in your repo 😊

image

Looking at how fast and efficient (CPU, memory) Moonfire-NVR is compared to all other Python/JS based NVRs that use ffmpeg I've tried so far I can't stop thinking that it's the way to go, but most of my cameras are Reolink and full-res max bitrate 4K streams are HEVC (H.265)…

Thank you folks for your work!

xnorpx commented 6 months ago

I would like to ditch my blue iris server and switch over to something driven by retina and use str0m for webrtc proxy. But yeah need h265 for that.

nemosupremo commented 6 months ago

Time flies; I'm going to try and get this done the only thing that's been holding it back is retina is a particularly high quality code base and we took some shortcuts in getting this done.

  1. We ended up leveraging https://github.com/quietvoid/hevc_parser. I'm waiting on the owner or that library to merge and publish a crate so that I don't have to. I was looking into dropping this dependency as the message on the label gave me the impression the owner didn't care about supporting it. However we've been using the library for a while and rewriting portions of it wasn't desirable.
  2. For parsing the streams we reused bitstream-io/bitvec_helpers that was used by hevc_parser. I only bring this up as the rest of the retina library seems to do bit twiddling manually.

Once hevc_parser is published and we can remove the references to github repos in our Cargo.toml I will isolate the h265 changes and open a repo here.

Sculas commented 6 months ago

Given that hevc_parser is MIT-licensed, if the original author doesn't respond within, let's say, a week, I think you should be okay to fork it and publish it yourself with the correct credit where it is due. You can publish it under a different name or, if I recall correctly, with the same name, but allow the original author to take the package over if they want to.

scottlamb commented 6 months ago

Looking forward to your PR. There are several Moonfire users who have been wanting H.265 support, even considering the caveats about browser support. I'd definitely give it a spin myself; some of my cameras do H.265...

the only thing that's been holding it back is retina is a particularly high quality code base

😊

we took some shortcuts in getting this done.

fwiw, I skimmed and think it looks pretty good. There are always things that can be tweaked, but they don't have to be considered blockers for merging. The one thing I'd really like to have is testing—both some basic functional tests and a fuzz test. (If you've never fuzzed before, you're in for a treat. It's surprisingly easy to do and really helps shake out problems.)

Also, an intermediate option might be to merge it but keep the functionality behind an experimental feature flag.

hevc_parser

Neat, I hadn't seen that crate. Fine with me to use in any way really: if the author publishes, if you publish a fork, or by vendoring the parts needed within Retina. It looks like you're not using a whole lot of functionality, and I'm not sure you even need quite all you are using. (In particular, I don't think it's necessary to assemble the Annex-B form and then parse it; the boundaries are already known so the nals could be directly fed to {VPS,NAL,SPS}NAL::parse instead.)

I think if I'd done this I would have tried adding H.265 to support to the h264-parser crate, as the two are really quite similar. (Maybe even with a rename to h2645-io or h26x-io; the -io suffix because I've also been interested in writing modified versions of the parameter sets to add in pixel aspect ratios, which several cameras annoyingly don't include.) But using the already-existing hevc-parser is fine, and we could always switch later.

For parsing the streams we reused bitstream-io/bitvec_helpers that was used by hevc_parser. I only bring this up as the rest of the retina library seems to do bit twiddling manually.

Parts do bit twiddling manually, like this RTP code, but they only have to handle fixed-width aligned fields.

For the more sophisticated variable-length field stuff, Retina is already using bitstream-io, both within the aac codec logic here and indirectly via h264-parser.

I hadn't seen bitvec_helpers. It looks redundant with h264_reader::rbsp::BitReader but I don't think that's a blocker. It's small, and again we could always switch later.

scottlamb commented 6 months ago

or by vendoring the parts needed within Retin

Although I guess there's the license: long-term I'd like to keep Retina on the dual Apache/MIT that seems to be the standard in the Rust ecosystem, and Sculas pointed out hevc_parser is (just) MIT. If the hevc_parser's author doesn't want to broaden to include Apache, we could still vendor temprarily by marking these parts as MIT-only and marking the crate overall as MIT-only in Cargo.toml as long as they're present.