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

webOS 4032 error in compiled version but not debug version #6533

Closed
stuartflanagan opened this issue May 5, 2024 · 22 comments · Fixed by #6558
Closed

webOS 4032 error in compiled version but not debug version #6533

stuartflanagan opened this issue May 5, 2024 · 22 comments · Fixed by #6558
Labels
flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this platform: WebOS Issues affecting WebOS priority: P1 Big impact or workaround impractical; resolve before feature release type: bug Something isn't working correctly
Milestone

Comments

@stuartflanagan
Copy link

Have you read the FAQ and checked for duplicate open issues?
Yes

What version of Shaka Player are you using?

4.8.x

Can you reproduce the issue with our latest release version?
Yes

Can you reproduce the issue with the latest code from main?
Yes

Are you using the demo app or your own custom app?
Both

If custom app, can you reproduce the issue using our demo app?
Yes

What browser and OS are you using?
LG webOS

For embedded devices (smart TVs, etc.), what model and firmware version are you using?
webOS3-2024

What are the manifest and license server URIs?
webOS is returning 4032 for manifest media types
HLS: https://demo.unified-streaming.com/k8s/features/stable/usp-s3-storage/tears-of-steel/tears-of-steel.ism/.m3u8
MPD: https://storage.googleapis.com/shaka-demo-assets/angel-one/dash.mpd

What configuration are you using? What is the output of player.getConfiguration()?
Default

What did you do?
Attempted to playback DASH and HLS manifest

What did you expect to happen?
Playback to occur

What actually happened?
Playback results in startup error with code 4032.
When switching to debug player shaka-player.compiled.debug.js
Playback is fine and error is not reported.

Playback for any stream type is failing for all webOS devices using the compiled version of Shaka Player 4.8 +
When attempting to debug the issue with the debug version the error is no longer there and playback is as expected.

@stuartflanagan stuartflanagan added the type: bug Something isn't working correctly label May 5, 2024
@shaka-bot shaka-bot added this to the v4.9 milestone May 5, 2024
@avelad avelad added flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this priority: P1 Big impact or workaround impractical; resolve before feature release platform: WebOS Issues affecting WebOS labels May 6, 2024
@avelad
Copy link
Collaborator

avelad commented May 6, 2024

We do not have a WebOS device to test, if you identify the PR that includes the regression, we can try to find the problem. Thank you!

@stuartflanagan
Copy link
Author

stuartflanagan commented May 6, 2024

Noted @avelad it is a bit tricky due to the nature of the issue.
Issue is only present in compiled version it affects all stream types.
Is there any guidance on what might have caused a regression in 4.8 that would cause webOS to fail only In compiled version?
Both debug and uncompiled perform as expected.
I was hoping the issue affecting us would be fixed in 4.7 as well which is the HLS AES over requesting as this affects all platforms including Samsung and web which would mean we are not reliant on upgrading to 4.8

@Illmatikx
Copy link

We have the same issue: 4032 with compiled version on webOS, but ok on Tizen and with debug version on webOS.

@avelad
Copy link
Collaborator

avelad commented May 6, 2024

Since it happens to many people, anyone who wants to contribute to fixing it, please do, we don't have WebOS in the lab to test. Thank you very much to all!

@stuartflanagan
Copy link
Author

This issue is a bit of a needle in a haystack.
It also puts a pretty large stop to any support for webOS altogether as it affects all webOS regardless of stream type.
The fact that the issue is gone in any debug or uncompiled version makes logging anything quite difficult.
@avelad is there any PR you can think of that would cause such a dramatic effect for webOS?
🙏

@avelad
Copy link
Collaborator

avelad commented May 7, 2024

Since it only happens in WebOS I don't know what to say...

@avelad
Copy link
Collaborator

avelad commented May 7, 2024

Since almost all bufixes are ported to other branches, the ideal would be to start with features: https://github.com/shaka-project/shaka-player/releases/tag/v4.8.0

@stuartflanagan
Copy link
Author

Seems to be occurring around the preload area maybe related to
#6271

@stuartflanagan
Copy link
Author

preloadManager.start() is failing but only when compiled and not debug 🤔

@stuartflanagan
Copy link
Author

Looking more into: feat: Add preload system to player #5897

@stuartflanagan
Copy link
Author

Hopefully create a PR tomorrow.
For some reason in compiled version manifest filter is checking variants
allowedByApplication is false for all variants.
When using uncompiled or debug allowedByApplication is true.

@avelad
Copy link
Collaborator

avelad commented May 7, 2024

@stuartflanagan can you review if #6180 introduce any error? Thanks!

@stuartflanagan
Copy link
Author

stuartflanagan commented May 8, 2024

Hi @avelad.
I have found the source of the issue and it is part of PR #6180
For some reason a compiled version accessing
JSON.parse returns undefined when accessing the properties.
But the Debug and Uncompiled version has access to the Parsed properties.

  const parsedDeviceInfo =
    /** @type {{screenWidth: number,screenHeight: number}} */
    (JSON.parse(deviceInfo));
  const screenWidth = parsedDeviceInfo.screenWidth;  // Is undefined in compiled Version
  const screenHeight = parsedDeviceInfo.screenHeight;  // Is undefined in compiled Version

I do not think this is a webOS issue but is potentially a compiler issue with JSON.parse?
Logging the parsed object shows all the properties and values seem good.
Accessing the property returns undefined, but only in non debug mode. 🤔

@joeyparrish
Copy link
Member

Ah, I see. JSON.parse returns the correct thing in every mode, but the compiler sees the type as something it can minify. So the compiled version would be something like:

const a = JSON.parse(window.PalmSystem.deviceInfo);
const b = a.b;
const c = a.c;

The solution is to move the parsed type to the externs or to use ['screenWidth'] to access instead of .screenWidth.

@stuartflanagan
Copy link
Author

Thanks @joeyparrish that seems a lot easier than the REGEX I have just created to parse the deviceInfo 🤣
Any issue with using array notation over dot notation in this scenario? I am not sure how to access the externs.

@stuartflanagan
Copy link
Author

Any chance this fix could go into an earlier release than 4.9?

@avelad
Copy link
Collaborator

avelad commented May 8, 2024

Once the PR is merged it, I'll release 4.8.3 asap.

avelad added a commit that referenced this issue May 8, 2024
avelad added a commit that referenced this issue May 8, 2024
@stuartflanagan
Copy link
Author

stuartflanagan commented May 8, 2024

@avelad & @joeyparrish I just had a question regarding this issue.
I saw Chromecast defaults to 720 in a new PR.
webOS does have devices with resolution of 720 out there as well. Should we drop default from 1080 to 720 for webOS as well?
I actually have a webOS 720 device here and playback is working as expected just not 100% sure 1080 minimum is safe.

@stuartflanagan
Copy link
Author

Sorry to comment on this closed issue. Should I create
Maybe suggest for next PR something more in line with Tizen?

// WebOS has always been able to do 1080p.  Assume a 1080p limit.
      // Set reasonable defaults in case an issue occurs
      maxResolution.width = 1920;
      maxResolution.height = 1080;
      try {
        const deviceInfo =
        /** @type {{screenWidth: number, screenHeight: number}} */(
            JSON.parse(window.PalmSystem.deviceInfo));
        // WebOS has always been able to do 1080p.  Assume a 1080p limit.
        const screenWidth = deviceInfo['screenWidth'];
        const screenHeight = deviceInfo['screenHeight'];

        if(!isNaN(screenWidth) && !isNaN(screenHeight)){
          maxResolution.width =  screenWidth;
          maxResolution.height = screenHeight;
        }
      } catch (e) {
        shaka.log.alwaysWarn('WebOS: Error detecting screen size, default ' +
            'screen size 1920x1080.');
      }

Hisense also has 720 devices too.

@avelad
Copy link
Collaborator

avelad commented May 9, 2024

Sorry, I don't think it's correct in the case of WebOS, looking at the specifications all versions since WebOS 3.0 support a minimum of 1080p: https://webostv.developer.lge.com/develop/specifications/video-audio-30

@avelad
Copy link
Collaborator

avelad commented May 9, 2024

I can't say anything about the Hisense case, because I can't test it right now and I don't have access to its documentation, but I think it's the same case with WebOS. This is normally due (at least in Europe), because even if the TV is 720, the broadcast broadcasts can be at 1080 and have to support them.

@stuartflanagan
Copy link
Author

Thank you for all your help with this @avelad!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this platform: WebOS Issues affecting WebOS priority: P1 Big impact or workaround impractical; resolve before feature release type: bug Something isn't working correctly
Projects
None yet
5 participants