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

feat(android): Support Common Media Client Data (CMCD) #4034

Merged
merged 17 commits into from
Aug 22, 2024

Conversation

uncoolclub
Copy link
Contributor

Summary

This PR adds support for CMCD (Common Media Client Data) in the ExoPlayer implementation of react-native-video. CMCD is a standard that allows clients to communicate QoE metrics and other contextual data to CDN providers and origin servers, enabling better analytics and potentially improved streaming performance.

Motivation

CMCD support is becoming increasingly important for video streaming applications, as it allows for better analytics and potential optimizations on the server side. By implementing CMCD in react-native-video, we enable developers to take advantage of these benefits in their React Native applications.

Changes

  1. Added CMCDConfig class to handle CMCD configuration.
  2. Updated ReactExoplayerView to support setting CMCD configuration.
  3. Modified ReactExoplayerViewManager to handle CMCD prop from React Native.
  4. Updated interface to allow setting CMCD configuration.

Test plan

To test this change:

  1. Set up a React Native project with react-native-video.
  2. Use the following code to enable CMCD:
       <Video
           source={{
               uri: 'https://demo.unified-streaming.com/k8s/features/stable/video/tears-of-steel/tears-of-steel-en.ism/.mpd',
               cmcd: true // or other custom cmcd configuration props...
           }}
        />
  3. Use a network inspector to verify that CMCD data is being sent along with video requests.
  4. If possible, use a server that supports CMCD to verify that the data is being received and processed correctly.

Test

1. CMCD Disabled (Default Behavior)

Test Case

  • Set cmcd: false or omit the cmcd property

Code

<Video
  source={{
    uri: 'https://demo.unified-streaming.com/k8s/features/stable/video/tears-of-steel/tears-of-steel-en.ism/.mpd',
    cmcd: false // or omit this line
  }}
/>

Result

CMCD was not activated, as confirmed in Android Studio's App Inspection > Network Inspector.

스크린샷 2024-07-25 오후 2 27 07

2. Default CMCD Configuration

Test Case

  • Set cmcd: true

Code

<Video
  source={{
    uri: 'https://demo.unified-streaming.com/k8s/features/stable/video/tears-of-steel/tears-of-steel-en.ism/.mpd',
    cmcd: true
  }}
/>

Result

CMCD was activated with default settings (query parameters), as verified in Android Studio's App Inspection > Network Inspector.

스크린샷 2024-07-25 오후 2 29 40

3. Custom CMCD Configuration with Request Header Mode

Test Case

  • Use custom CMCD configuration
  • Set mode: CmcdMode.MODE_REQUEST_HEADER

Code

<Video
  source={{
    uri: 'https://demo.unified-streaming.com/k8s/features/stable/video/tears-of-steel/tears-of-steel-en.ism/.mpd',
    cmcd: {
      mode: CmcdMode.MODE_REQUEST_HEADER,
      object: {
        'video-id': '1234',
      },
      session: {
        'session-test-id': 12,
      },
    }
  }}
/>

Result

Custom CMCD configuration was successfully applied using the request header mode.

스크린샷 2024-07-25 오후 2 32 30

4. Custom CMCD Configuration with Default Mode

Test Case

  • Use custom CMCD configuration
  • Omit the mode property (should default to query parameters)

Code

<Video
  source={{
    uri: 'https://demo.unified-streaming.com/k8s/features/stable/video/tears-of-steel/tears-of-steel-en.ism/.mpd',
    cmcd: {
      object: {
        'video-id': '1234',
      },
      session: {
        'session-test-id': 12,
      },
    }
  }}
/>

Result

Custom CMCD configuration was successfully applied using the default query parameter mode.

스크린샷 2024-07-25 오후 2 34 37

…VideoSrc type to enable cmcd feature on android

feat(video.ts): introduce CmcdMode enum and CmcdConfiguration type to define cmcd configuration options
…t to handle Content Management and Delivery (CMCD) headers for Android platform.
…ties and parsing logic for React Native module
…n for ExoPlayer with support for custom data and configuration options.
…ion in ReactExoplayerViewManager to enable Content Management and Control Data (CMCD) for better video playback optimization.
…ion.Factory to customize CMCD configuration for media playback
… the component, including usage examples and default values
@uncoolclub uncoolclub marked this pull request as ready for review July 25, 2024 07:31
Copy link
Member

@KrzysztofMoch KrzysztofMoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 left two small comments - Thanks for opening this PR

@YangJonghun cloud you also review this PR? @freeboub is on vacation and I would like also to someone else to review

docs/pages/component/props.mdx Outdated Show resolved Hide resolved
@YangJonghun
Copy link
Collaborator

@KrzysztofMoch I've already reviewed this PR :)

… prevCmcdConfig variables to clean up code and improve readability
feat(props.mdx): add definitions for CmcdMode enum and CmcdData type to enhance understanding of CMCD data structure and usage
@seyedmostafahasani
Copy link
Collaborator

seyedmostafahasani commented Jul 25, 2024

I have a suggestion. I think we should change the CMCDProps to a data class for better data representation and automatic generation of equals and hashCode methods.
WDYT? @YangJonghun @KrzysztofMoch

…proved readability and immutability

-  update CMCDProps class to use List instead of Array for properties
@uncoolclub
Copy link
Contributor Author

@seyedmostafahasani

488f498

Hi, Thank you for the great review. As you suggested, I have changed it to a data class.

@seyedmostafahasani
Copy link
Collaborator

seyedmostafahasani commented Jul 26, 2024

@uncoolclub thanks for applying my suggestion.
I had another suggestion. Did you see it, and what do you think about it?

@YangJonghun
Copy link
Collaborator

YangJonghun commented Jul 26, 2024

@seyedmostafahasani
Agree your suggestion that change to data class.
But I can't see any other suggestions, could you explain again what is it?

Copy link
Collaborator

@freeboub freeboub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 small comments, but it looks great 👍

@seyedmostafahasani
Copy link
Collaborator

@seyedmostafahasani Agree your suggestion that change to data class. But I can't see any other suggestions, could you explain again what is it?

My suggestion is regarding _cmcd in Video.tsx.
@uncoolclub @YangJonghun

@uncoolclub
Copy link
Contributor Author

uncoolclub commented Jul 26, 2024

My suggestion is regarding _cmcd in Video.tsx.
@uncoolclub @YangJonghun

@seyedmostafahasani
I can't see the suggestion you left about _cmcd in Video.tsx in the comments. It seems like it might not have been sent. Could you check again?

If it's in the comments, I can't find it, so could you please provide the suggestion again?

src/Video.tsx Outdated Show resolved Hide resolved
…ReactExoplayerView component

feat(ReactExoplayerViewManager.java): remove redundant CMCD configuration logic from ReactExoplayerViewManager to simplify code and improve maintainability
src/Video.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@freeboub freeboub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One remaining comment, can you please check it. After that It will be ok for me !
Thank you for the PR !

@seyedmostafahasani
Copy link
Collaborator

@uncoolclub,
There are conflicts that need to be resolved.

@uncoolclub
Copy link
Contributor Author

@seyedmostafahasani1e9c258
Resolved conflicts with the master branch

@freeboub
Copy link
Collaborator

@KrzysztofMoch this PR can also be merged I think

@KrzysztofMoch KrzysztofMoch merged commit ca795f2 into TheWidlarzGroup:master Aug 22, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants