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

[FragmentedSampleReader] Check default_isProtected before add senc #1710

Merged
merged 1 commit into from
Nov 2, 2024

Conversation

CastagnaIT
Copy link
Collaborator

@CastagnaIT CastagnaIT commented Oct 29, 2024

Description

PR #1688 introduced a kind of workaround to add SENC box when missing
this caused a kind of problem with smoothstreaming

smoothstreaming as usual is a bit shitting format
since dont have initialization segment a custom MOOV box is created manually from

AP4_Movie* PLAYLIST::CreateMovieAtom(adaptive::AdaptiveStream& adStream,
kodi::addon::InputstreamInfo& streamInfo)
{

the problem is a bit wide, i think that FragmentedSampleReader code can be potentially improved
but its impossible to me make a good code cleanup and or a better fix
since atm i dont have at my hand all streams that i can use to test all use cases

this part of code is as cat and mice

// If the boxes saiz, saio, senc are missing, the stream does not conform to the specs and
// may not be decrypted, so try create an empty senc where all samples will use the same default IV
if (!traf->GetChild(AP4_ATOM_TYPE_SAIO) && !traf->GetChild(AP4_ATOM_TYPE_SAIZ) &&
!traf->GetChild(AP4_ATOM_TYPE_SENC))
{
traf->AddChild(new AP4_SencAtom());
}
bool reset_iv(false);
if (AP4_FAILED(result = AP4_CencSampleInfoTable::Create(m_protectedDesc, traf, algorithm_id,
reset_iv, *m_FragmentStream,
moof_offset, sample_table)))
// we assume unencrypted fragment here
goto SUCCESS;
if (!m_singleSampleDecryptor)
return AP4_ERROR_INVALID_PARAMETERS;
m_decrypter = new CAdaptiveCencSampleDecrypter(m_singleSampleDecryptor, sample_table);

we need to check for missing SENC before call AP4_CencSampleInfoTable::Create to prevent that Create method to fails
but on Issue thread the smoothstreaming (playready) manifest converted to widevine, must not use CAdaptiveCencSampleDecrypter,
since the video stream of the Issue dont have SAIZ/SAIO/SENC, SENC is so created,
and if i understand correctly this seem to prevent the use of widevine decrypter from CFragmentedSampleReader::ReadSample()

as kind of solution i have set TENC default_isProtected parameter to 0 that means "not encrypted", this is not so clean solution, but i have no better ideas atm since i have no way to make tests for each use case

anyway TENC default_isProtected parameter is not so important to bento4 source code, and also we dont have code that depends from it, then should be a kind of solution that at least does not produce side effects

Sidenote: TENC default_isProtected on bento4 allow to have values different from 0 and 1, its weird,
looks like bento4 default_isProtected param support is a sort of custom implementation that dont follow the MP4 standard
at least im not able to find similar specs

Motivation and context

fix #1708

How has this been tested?

user tested his SS video
and some segments are here IssueSegments.zip

another use case is from sample provided by PR #1688

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • I have read the Contributing document
  • My code follows the Code Guidelines of this project
  • My change requires a change to the Wiki documentation
  • I have updated the documentation accordingly

@CastagnaIT CastagnaIT changed the title Revert "[FragmentedSampleReader] Create senc when saio/saiz/senc are … [FragmentedSampleReader] Check default_isProtected before add senc Oct 30, 2024
zuzia-dev added a commit to zuzia-dev/Kodi-Piers-addons-for-Linux that referenced this pull request Oct 30, 2024
@CastagnaIT CastagnaIT marked this pull request as ready for review October 31, 2024 06:33
@CastagnaIT CastagnaIT added Type: Fix non-breaking change which fixes an issue v22 Piers labels Oct 31, 2024
@CastagnaIT
Copy link
Collaborator Author

@glennguy no idea if you are still around here, if you want to take a look here

@CastagnaIT CastagnaIT merged commit 397a0c1 into xbmc:Piers Nov 2, 2024
10 checks passed
@CastagnaIT CastagnaIT deleted the revert_senc branch November 2, 2024 13:58
@glennguy
Copy link
Contributor

glennguy commented Nov 3, 2024

Sorry @CastagnaIT - yes still here :)
Looks like you've done a thorough investigation and without a big test suite all we can do is try and see how it goes

@CastagnaIT
Copy link
Collaborator Author

as i thought... you should try to make a list of test streams for each use case, a boredom
but ISA samples test addon could help to make a video list in a new menu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue v22 Piers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] [ism] problems with playback from v21.5.5/22.1.6
2 participants