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

playback issue of HEVC content with CENC cbcs encryption in AVplayer #717

Closed
jingke2000 opened this issue Feb 26, 2020 · 45 comments
Closed
Labels
flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@jingke2000
Copy link

System info

Operating System: macOS High Sierra, V10.13.4
Shaka Packager Version: version v2.3.0-5bf8ad5-release
playback device: iPad with iOS 13.3.1

Issue and steps to reproduce the problem

We have 4K video conent encoded in HEVC packaged in HLS fmp4. The playback from the AVplayer in an iPad with iOS 13.3.1 is fine. However, after CENC cbcs encryption, the playback shows macroblocking issue every 1 or 2seonds. We used Shaka Packager to package or encrypt the 4K content in both cases. Here are the links to the sample conent:
Clear (playback is OK):
https://hls-demo-s3.s3.us-east-2.amazonaws.com/4k_cbcs_testing/3/video.m3u8
Encrypted (playback has macroblocking issue):
https://hls-demo-s3.s3.us-east-2.amazonaws.com/4k_cbcs_testing/4/video.m3u8

Packager Command:
"
packager-osx 'in=PEDRO_E_INES_TRAILER_4KUHD_PRORES_04022019_UHD20_L1.mp4 ,stream=video,init_segment=4k_cbcs_shaka_clr_key/video/init.mp4,segment_template=4k_cbcs_shaka_clr_key/video/$Number$.m4s,playlist_name=video.m3u8,iframe_playlist_name=iframe.m3u8' 'in=PEDRO_E_INES_TRAILER_4KUHD_PRORES_04022019_UHD20_L1.mp4,stream=audio,init_segment=4k_cbcs_shaka_clr_key/audio/init.mp4,segment_template=4k_cbcs_shaka_clr_key/audio/$Number$.m4s,playlist_name=audio.m3u8,hls_group_id=audio,hls_name=ENGLISH' --protection_scheme cbcs --enable_raw_key_encryption --keys key_id=55fa9a934d7f3cbebf99b644612598c9:key=03030303030303030303030303030303 --protection_systems FairPlay --iv CC06DA420052BA12925E96EBF98C2F4E --hls_master_playlist_output 4k_cbcs_shaka_clr_key/master.m3u8 --hls_key_uri <key_url>
"

What is the expected result?
Playback of the CENC cbcs encrypted 4K content without visual defects;
What happens instead?
Playback of the CENC cbcs encrypted 4K content shows macroblocking issue every 1~2 seconds;

Source Content
Source content can be downloaded here:
https://hls-demo-s3.s3.us-east-2.amazonaws.com/4k_cbcs_testing/PEDRO_E_INES_TRAILER_4KUHD_PRORES_04022019_UHD20_L1.mp4

@kqyang
Copy link
Collaborator

kqyang commented May 6, 2020

@astinus-1 Can you try if we can re-pro?

@avrecko
Copy link
Contributor

avrecko commented May 11, 2020

I compared cbcs result of Shaka packager against another packager.

There are occasional differences in detected hevc slice header lengths. I suspect your hevc slice length parsing code has a bug.

You might want to compare your hevc slice parsing results against ffmpeg.

@kqyang
Copy link
Collaborator

kqyang commented May 11, 2020

@avrecko Thanks for doing the comparison. Would you mind sharing one or a few slice headers that have different results? That will help us identify the problem sooner.

@kqyang kqyang added type: bug Something isn't working correctly flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this and removed needs triage labels May 11, 2020
@shaka-bot shaka-bot added this to the v2.5 milestone May 11, 2020
@avrecko
Copy link
Contributor

avrecko commented May 12, 2020

@kqyang sure thing. I created a comparison file. This is from my own sample file and not the one linked in the ticket. Available here:

https://gist.github.com/avrecko/404b5cff4f79f4fc82e3dc33753de637

The values in the file should be self explanatory. I suggest you include some of this in your unit tests. Happy hunting.

@kqyang
Copy link
Collaborator

kqyang commented May 12, 2020

@avrecko Thanks very much for your help! Can you include codec configuration record which includes SPS and PPS in the gist too?

@avrecko
Copy link
Contributor

avrecko commented May 12, 2020

@kqyang Maybe you already figured it out. But if you look closely, at the arbitrarily chosen 256 bytes, the first sample is an IDR. It has aud, vps, sps, pps, 3x sei and then finally the partial slice. It should have 95 bytes of slice nal left. This is enough bytes to be sure that the slice header is complete.

You can use this data to test your nal parsing, vps, sps, pps parsing and slice header parsing. There are a few more IDR frames together with non-IDR frames in the gist. You should use the vps/sps/pps from the first frame until another IDR frames comes along.

@jingke2000
Copy link
Author

Thanks @avrecko and @kqyang for triaging on this issue. Is there any activity underway on fixing it? I saw it was included in V2.5. Does that imply the fix (with many others) to be released on 08/31?

@kqyang
Copy link
Collaborator

kqyang commented May 14, 2020

@jingke2000 Yes, that is the plan. As for this issue, we'll try to address it this month.

@jingke2000
Copy link
Author

@kqyang Thanks for the update. From the analysis by @avrecko, there seem to be occasional mismatches of H.265 slice header size. Have you found the root cause of it?

@kqyang
Copy link
Collaborator

kqyang commented May 14, 2020

No. We have not looked into it yet.

@jingke2000
Copy link
Author

OK. I can also do some analysis toward it and will report back here with any of my findings.

@kqyang
Copy link
Collaborator

kqyang commented May 14, 2020

@jingke2000 That will be great. Thanks. We welcome pull request too! If you find out where the problem is, you are very welcomed to submit a PR to us to fix it.

@jingke2000
Copy link
Author

Sure. Will do.

@avrecko
Copy link
Contributor

avrecko commented May 16, 2020

Gpac might not be the best reference to compare with. I've just debugged Gpac code by looking what it does with the first P frame. It creates a pair 32|x while Shaka 48|x and an in-house solution I am working on 48|x. Also looked at the FFmpeg hevc slice structure state and indeed it matches with our own.

This is the line where the parsing becomes incorrect in GPac
if (pps->lists_modification_present_flag /*TODO: && NumPicTotalCurr > 1*/)

I'll look what is going on with the rest of the frames soon. Will keep you posted.

@kqyang
Copy link
Collaborator

kqyang commented May 16, 2020

@avrecko Thanks! Btw, have you tested the stream generated by Gpac, does it play correctly in AVPlayer?

@avrecko
Copy link
Contributor

avrecko commented May 17, 2020

@kqyang Not yet. First I want to verify that the pairs are correct. I am cross comparing the slice header parsing with FFmpeg. I'll post results soon.

@avrecko
Copy link
Contributor

avrecko commented May 19, 2020

I deleted the gist with comparison with Gpac. It is useless as in my testing and comparison with openHEVC (FFmpeg with just the hevc part) they are both inaccurate (edit: Both Shaka and Gpac).

Here is one difference I found when comparing Shaka output, my own hevc parser and openHEVC.

edit: deleted the gist as this sample works correctly.

Please verify that you respect the following in your code:

  • When collocated_from_l0_flag is not present, it is inferred to be equal to 1.
  • When the current slice is a P or B slice and num_ref_idx_l0_active_minus1 is not present, num_ref_idx_l0_active_minus1 is inferred to be equal to num_ref_idx_l0_default_active_minus1. (applies for l1 as well).

@kqyang
Copy link
Collaborator

kqyang commented May 19, 2020

Verify that the slice header length of the following is 23. Afaik Shaka sees 24.

@avrecko Do you infer Shaka's slice header length through subsample size? That is adjusted by rounding cipher size to multiple of 16 bytes.

When collocated_from_l0_flag is not present, it is inferred to be equal to 1.

Yes: https://github.com/google/shaka-packager/blob/master/packager/media/codecs/h265_parser.h#L263

When the current slice is a P or B slice and num_ref_idx_l0_active_minus1 is not present, num_ref_idx_l0_active_minus1 is inferred to be equal to num_ref_idx_l0_default_active_minus1. (applies for l1 as well).

Yes: https://github.com/google/shaka-packager/blob/master/packager/media/codecs/h265_parser.cc#L296.

@avrecko
Copy link
Contributor

avrecko commented May 19, 2020

@kqyang For the sample in gist. Shaka generated a pair P(49|X) while the in-house cbcs implementation I just finished P(48|X). I verified the hevc slice length using openHEVC for all of the samples in the sample video. For this particular sample in gist the length should be 23. I suspect that Shaka is seeing a different slice header length. Would just like to know if that is the case.

Now for the fun part. I'll be playing shaka, gpac and the in-house cbcs packaged videos on some devices.

@kqyang
Copy link
Collaborator

kqyang commented May 19, 2020

I had an incorrect statement in my earlier comments:

That is adjusted by rounding cipher size to multiple of 16 bytes.

This is incorrect. The subsample size is not adjusted by cbcs, so there is definitely some difference between SHAKA slice header parser and your in-house cbcs parser.

I'll be playing shaka, gpac and the in-house cbcs packaged videos on some devices.

Great. Keep us updated.

@avrecko
Copy link
Contributor

avrecko commented May 20, 2020

@kqyang I've updated the in-house cbcs support. Now the results are a bit different.

I have 5 production hevc streams as samples. In automated testing comparison between in-house cbcs and shaka. 3 streams match with the lengths. Including the sample from the gist. I played the sample from the gist successfully on Safari. Both in-house and Shaka play without visual glitches, GPac fails to play on Safari.

I'll look into the difference with the other 2 streams soon. Ball parking it. The difference is a few frames per 100 frames. In one stream the differences in plain length are pretty substantial. The other stream differs in 1 byte plain length. In 3 out of 5 streams there are no differences so this is very good start.

Looking forward to get to the bottom of this.

@avrecko
Copy link
Contributor

avrecko commented May 21, 2020

@kqyang I have a sample made with the in-house cbcs and one with shaka. With shaka there are some visual glitches while with the in-house there is smooth playback on Safari.

I've extracted the first frame where in-house and shaka differ in plain length. 40 pain bytes for Shaka vs 36 bytes for in-house.

https://gist.github.com/avrecko/0ce67d5a570f5b9bb8deed7f54968d64

What is your slice header length calculation?

@kqyang
Copy link
Collaborator

kqyang commented May 21, 2020

@avrecko Great. Thanks for nail down the problematic slice!

Here is our slice header parsing logic:

https://github.com/google/shaka-packager/blob/ab8fa87d1807e2f8a5756e365f9bf7b41cc4a992/packager/media/codecs/h265_parser.cc#L183

Can you help us check where the problem is?

@avrecko
Copy link
Contributor

avrecko commented May 22, 2020

@kqyang I found the problem. Made a fix. But will make a pull request when I track down one more difference between shaka and in-house.

avrecko@a4362bb

Basically you always use default value while it can be set explicitly in the slice header.

@avrecko
Copy link
Contributor

avrecko commented May 22, 2020

@jingke2000 I looked at your sample. You have 2x vps,sps and pps per irap frame. Are you sure this makes sense?

@avrecko
Copy link
Contributor

avrecko commented May 22, 2020

@kqyang There is at least 1 more problem after the fix is applied. With in-house cbcs it plays fine on Safari. Shaka has glitches. Occasionally Shaka makes a slightly larger plain size for irap frames before the fix the differences were all over the place.

@jingke2000
Copy link
Author

@avrecko You are right. That doesn't make sense. We changed it to single set of vps/sps/pps. I'll capture and upload a correct sample here.

Thanks!

@kqyang
Copy link
Collaborator

kqyang commented May 22, 2020

Re avrecko/shaka-packager@a4362bb, great finding! Thanks.

There is at least 1 more problem after the fix is applied. With in-house cbcs it plays fine on Safari.

@avrecko Will you be able to help us track it down as well? Appreciated!

@avrecko
Copy link
Contributor

avrecko commented May 22, 2020

@kqyang I found the problem but you will have to fix it.

The EOS_NUT unit is added at the start of the next frame instead at the end of the previous frame. This looks to be a generic problem unrelated to cbcs? New issue? Except for this Shaka cbcs works with the samples I tested on.

@kqyang
Copy link
Collaborator

kqyang commented May 22, 2020

@avrecko Great that you found the problem.

The EOS_NUT unit is added at the start of the next frame instead at the end of the previous frame

Can you be more specific? I don't think we would add NAL units to the frame.

@avrecko
Copy link
Contributor

avrecko commented May 22, 2020

@kqyang I am using the word frame a bit loosely. I was not referring to slice if that is what you mean. I had in mind the whole sample.

Maybe this will be more helpful:

SubsampleGenerator::GenerateSubsamplesFromH26xFrame
     ....
     while ((result = reader.Advance(&nalu)) == NaluReader::kOk) {
        // &nalu should change with iterating to AUD, VPS, SPS, PPS, Slice
        // but is like this: EOS_NUT, AUD, VPS, SPS, PPS, Slice
     }

avrecko added a commit to avrecko/shaka-packager that referenced this issue May 22, 2020
avrecko added a commit to avrecko/shaka-packager that referenced this issue May 22, 2020
@kqyang
Copy link
Collaborator

kqyang commented May 22, 2020

SubsampleGenerator::GenerateSubsamplesFromH26xFrame does not actually change the sample. It parses the samples and determine subsample boundary. It will be more helpful if you can give an example with data like what you did earlier.

@avrecko
Copy link
Contributor

avrecko commented May 22, 2020

I'll prepare a sample soon. Looking at ts file:

Sample N: has AUD, Prefix SEI, Trail_R, EOS_NUT
Sample N+1: AUD, VPS, SPS, PPS,...

While for Shaka:
Sample N + 1: EOS_NUT, AUD,...

@avrecko
Copy link
Contributor

avrecko commented May 23, 2020

@kqyang Regarding EOS_NUT. The problem is in the mp4 file and not in Shaka.

What happened is this:

ffmpeg -i InputWithEosAtTheEndOfPesPacket.ts -c copy Mp4WithEosNalMovedToStartOfSample.mp4

I did not expect this from ffmpeg. Now the 5 samples I have all match with Shaka.

@kqyang
Copy link
Collaborator

kqyang commented May 23, 2020

I see. Thanks for checking.

avrecko added a commit to avrecko/shaka-packager that referenced this issue May 24, 2020
@kqyang kqyang closed this as completed in 15934d6 May 24, 2020
@kqyang
Copy link
Collaborator

kqyang commented May 24, 2020

Thanks @avrecko for addressing the issue.

@jingke2000 You may compile Shaka Packager with 15934d6, or wait for Shaka Packager v2.4.3 release, which should happen next week. Let us know if you are still experiencing playback problems with the fix.

@jingke2000
Copy link
Author

Thanks @avrecko @kqyang. You guys are moving super fast. Yes I'll get code fixed and test it with our transcoders. I'll post the result here.

@avrecko
Copy link
Contributor

avrecko commented May 25, 2020

@jingke2000 Out of curiosity. Could you share a few details about your transcoder? Do you use ffmpeg for the actual encoding or do you have an in-house hevc encoder?

I think that the biggest complexity in transcoding is in making a good balance between low delay and good compression. Would that be accurate? As for regular VOD you can do a 2 pass and have lookahead at 50-60 frames. Not something you can afford if you want to keep the delay low?

@jingke2000
Copy link
Author

@avrecko
Hi. Sorry I was not able to write back to you earlier.
We work with different transcoders (an in-house one and other commercial ones) as ingests for OTT ABR streaming products.
You are absolutely right. For linear live transcoding, a key aspect is to make good balance between low latency and good compression. To get good compression, we can buffer more (e.g., one GOP). But that would add at least one GOP duration of latency. To get lower latency, we can buffer less but compression would suffer and required bit rate would go up.
So end of the day it is all about tradeoffs between image quality, latency and bit rates.

@jingke2000
Copy link
Author

@avrecko @kqyang Hi, I finally got chance to test the fix. However, this fix doesn't solve the bug I ran into. I still see the same video artifacts every 1 -2 seconds.

For the sample: https://hls-demo-s3.s3.us-east-2.amazonaws.com/4k_cbcs_testing/PEDRO_E_INES_TRAILER_4KUHD_PRORES_04022019_UHD20_L1.mp4, function 'SkipReferencePictureListModification' is never called because the sample's 'pps->lists_modification_present_flag' is always 0. So the code changes in the function 'SkipReferencePictureListModification' have no impact on this sample's slice header parsing.

@avrecko Were you able to see the difference in the slice header or parameter sets between my sample above and yours?

@kqyang kqyang reopened this Jun 10, 2020
@avrecko
Copy link
Contributor

avrecko commented Jun 11, 2020

@jingke2000 I looked at your sample. You still have duplicated VPS, SPS and PPS entries. I don't think you should expect 3rd party software to work well with a sample like that.

I made ts from the provided file using ffmpeg copy codec. Shaka crashes on the ts file with:

[0611/154201:FATAL:h26x_bit_reader.cc(79)] Check failed: num_bits <= 31. 
#0 0x0000004a501e base::debug::StackTrace::StackTrace()
#1 0x00000044d07f logging::LogMessage::~LogMessage()
#2 0x000000f4d12a shaka::media::H26xBitReader::ReadBits()
#3 0x000000f34469 shaka::media::H265Parser::ParseSliceHeader()
#4 0x000000f5949c shaka::media::H265VideoSliceHeaderParser::GetHeaderSize()

Please provide sample with non duplicated VPS, SPS, PPS and I'll take a look again.

@avrecko
Copy link
Contributor

avrecko commented Jun 11, 2020

@jingke2000 Besides the duplicated stuff. I've decided to run this sample against the in-house cbcs packager. I just need to put it in the samples directory and run some scripts. On frame 105 it crashes with input exhausted.

I've dumped the offending frame to hevc file and ran it against libde265 and openHEVC. They both crash as well.

Libde265:
decoding error: coded parameter out of range (code=8).

openHEVC:
[hevc @ 0x1bcf8e0] Invalid number of merging MVP candidates: 0.
[hevc @ 0x1bcf8e0] Error parsing NAL unit #3.

I suggest you first verify that your output can be successfully decoded by libde265 or openHEVC.

A good test is also transcoding with ffmpeg. You can see that it reports errors in decode.

@kqyang I suggest you close this. We can reopen if @jingke2000 sample passes decoding by ffmpeg/libde265 without errors but shaka produces cbcs output that doesn't play well.

@jingke2000
Copy link
Author

@avrecko You are right. There are two sets of VPS/SPS/PPS in front of each IDR slice in my sample content. To make it worse, I found these two sets of parameter sets are not even the same. That would certainly cause trouble. So I agree this content is invalid.

@kqyang I made the statement of "this fix doesn't solve the bug I ran into" too early. Sorry about that. I'm working with the content provider to get it corrected. I'll test it again when they come back to me. I'll keep you updated.

@kqyang
Copy link
Collaborator

kqyang commented Jun 11, 2020

@avrecko Thanks for looking into it!

@jingke2000 Thanks for the update. I am closing this bug. Feel free to reply if there is an update.

@kqyang kqyang closed this as completed Jun 11, 2020
@kqyang
Copy link
Collaborator

kqyang commented Aug 1, 2020

Cherry-picked to v2.4.x branch for v2.4.3 release.

kqyang pushed a commit that referenced this issue Aug 1, 2020
Fixes #717: playback issue of HEVC content with cbcs encryption in AVplayer.
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Aug 10, 2020
@shaka-project shaka-project locked and limited conversation to collaborators Aug 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

4 participants