Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

h.265 support #57

Open
Sculas opened this issue Apr 16, 2022 · 12 comments
Open

h.265 support #57

Sculas opened this issue Apr 16, 2022 · 12 comments
Labels
enhancement New feature or request

Comments

@Sculas
Copy link

Sculas commented Apr 16, 2022

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 scottlamb added the enhancement New feature or request label Apr 27, 2022
@scottlamb
Copy link
Owner

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
Copy link
Contributor

@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
Copy link
Owner

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
Copy link
Contributor

@scottlamb fyi nemosupremo#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
Copy link

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
Copy link

xnorpx commented Dec 31, 2023

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
Copy link
Contributor

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
Copy link
Author

Sculas commented Dec 31, 2023

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
Copy link
Owner

scottlamb commented Jan 1, 2024

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
Copy link
Owner

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.

@scottlamb
Copy link
Owner

I have a work in progress for this on the h265 branch. I took inspiration from @nemosupremo's fork but didn't use the hevc_parser crate. It looked like that crate needed some work (no tests, panicked when I fuzz tested it), and I thought it'd be nice to reuse h264_reader::rbsp and match the feel of the h264-reader crate in general, so I rolled my own for that bit.

@tubzby
Copy link

tubzby commented Nov 23, 2024

Any news on this? H265 codecs can be enabled on recent Chrome by some feature flags, I think it might get stable soon.
I have checked the h265 branch, test it against a HikiVision camera with no luck:

I20241123 16:39:09.586 main client::mp4] Using h265 video stream
I20241123 16:39:09.646 main client::mp4] Ignoring audio streams (if any) because of --no-audio
I20241123 16:39:09.771 main client::mp4] writing MP4 failed; details will be logged with `Fatal:` after RTSP session teardown
E20241123 16:39:09.866 main client] Fatal: Out-of-order packet or large loss; expecting ssrc=Some(Ssrc { init: PlayResponseHeader, ssrc: 0x7ce036cb }) seq=Some(Seq { init: PlayResponseHeader, next: 19844 })

conn: 10.8.0.6:58013(me)->192.168.231.140:554@2024-11-23T16:39:09
stream: TCP, interleaved channel ids 0-1
ssrc: 7ce036cb
seq: 53953
pkt: 1509@2024-11-23T16:39:09

----EDIT 1-----

Sorry about that, seems the camera sent inconsistent nalu types within different Fu packages, I have ignored that error and it now works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants