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.

Sign up for GitHub

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

Implement SDL 0323: Align video streaming parameters with VideoStreamingCapability #1576

Merged
merged 10 commits into from
Jan 28, 2021
Merged

Implement SDL 0323: Align video streaming parameters with VideoStreamingCapability #1576

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7fb3515
Add missing stableFrameRate for default ctor.
shiniwat Jan 7, 2021
1e2b923
Fix a potential case where VideoFormat is unexpectedly overridden by …
shiniwat Jan 7, 2021
4b1ff6d
Remove unused VideoStreamingParameter ctor, and mark existing one dep…
shiniwat Jan 8, 2021
19e10a2
Update rpc_spec submodule for SDL 0274 reviewed at https://github.com…
shiniwat Jan 15, 2021
d37b8e5
Merge remote-tracking branch 'upstream/integration/stable_frame_rate'…
shiniwat Jan 22, 2021
c82f344
Update PR as per review comment at https://github.com/smartdevicelink…
shiniwat Jan 25, 2021
65b3011
Revert "Update rpc_spec submodule for SDL 0274 reviewed at https://gi…
shiniwat Jan 25, 2021
1d1eb83
Update VideoStreamingParameters#update, as per review at https://gith…
shiniwat Jan 27, 2021
9c6136a
Fix the potential integer overflow case when taking lower value of bi…
shiniwat Jan 28, 2021
4fa997a
Refix the possible overflow case in capability's max bitrate.
shiniwat Jan 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions android/sdl_android/src/main/java/com/smartdevicelink/encoder/VirtualDisplayEncoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,26 @@ public VideoStreamingParameters getStreamingParams() {
return this.streamingParams;
}

/**
* This method is deprecated; setStreamingParams with having stableFrameRate should be used.
*/
@Deprecated
public void setStreamingParams(int displayDensity, ImageResolution resolution, int frameRate, int bitrate, int interval, VideoStreamingFormat format) {
this.streamingParams = new VideoStreamingParameters(displayDensity, frameRate, bitrate, interval, resolution, format);
}

@SuppressWarnings("unused")
public void setStreamingParams(int displayDensity, ImageResolution resolution, int frameRate, int bitrate, int interval, VideoStreamingFormat format, boolean stableFrameRate) {
this.streamingParams = new VideoStreamingParameters(displayDensity, frameRate, bitrate, interval, resolution, format, stableFrameRate);
/**
* setter of every parameter in streamingParams.
* @param displayDensity
* @param resolution
* @param frameRate
* @param bitrate
* @param interval
* @param format
* @param stableFramerate
*/
public void setStreamingParams(int displayDensity, ImageResolution resolution, int frameRate, int bitrate, int interval, VideoStreamingFormat format, boolean stableFramerate) {
this.streamingParams = new VideoStreamingParameters(displayDensity, frameRate, bitrate, interval, resolution, format, stableFramerate);
}

shiniwat marked this conversation as resolved.
Show resolved Hide resolved
@SuppressWarnings("unused")
Expand Down
55 changes: 26 additions & 29 deletions android/sdl_android/src/main/java/com/smartdevicelink/managers/video/VideoStreamManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,38 +294,35 @@ public void startRemoteDisplayStream(Context context, Class<? extends SdlRemoteD
stateMachine.transitionToState(StreamingStateMachine.ERROR);
return;
}
if (parameters == null) {
if (majorProtocolVersion >= 5) {
if (internalInterface.getSystemCapabilityManager() != null) {
internalInterface.getSystemCapabilityManager().getCapability(SystemCapabilityType.VIDEO_STREAMING, new OnSystemCapabilityListener() {
@Override
public void onCapabilityRetrieved(Object capability) {
VideoStreamingParameters params = new VideoStreamingParameters();
params.update((VideoStreamingCapability) capability, vehicleMake); //Streaming parameters are ready time to stream
startStreaming(params, encrypted);
}
// regardless of VideoStreamingParameters are specified or not, we should refer to VideoStreamingCapability.
if (majorProtocolVersion >= 5) {
if (internalInterface.getSystemCapabilityManager() != null) {
final VideoStreamingParameters params = ( parameters == null) ? new VideoStreamingParameters() : new VideoStreamingParameters(parameters);
internalInterface.getSystemCapabilityManager().getCapability(SystemCapabilityType.VIDEO_STREAMING, new OnSystemCapabilityListener() {
@Override
public void onCapabilityRetrieved(Object capability) {
params.update((VideoStreamingCapability) capability, vehicleMake); //Streaming parameters are ready time to stream
startStreaming(params, encrypted);
}

@Override
public void onError(String info) {
stateMachine.transitionToState(StreamingStateMachine.ERROR);
DebugTool.logError(TAG, "Error retrieving video streaming capability: " + info);
}
}, false);
}
} else {
//We just use default video streaming params
VideoStreamingParameters params = new VideoStreamingParameters();
DisplayCapabilities dispCap = null;
if (internalInterface.getSystemCapabilityManager() != null) {
dispCap = (DisplayCapabilities) internalInterface.getSystemCapabilityManager().getCapability(SystemCapabilityType.DISPLAY, null, false);
}
if (dispCap != null) {
params.setResolution(dispCap.getScreenParams().getImageResolution());
}
startStreaming(params, encrypted);
@Override
public void onError(String info) {
stateMachine.transitionToState(StreamingStateMachine.ERROR);
DebugTool.logError(TAG, "Error retrieving video streaming capability: " + info);
}
}, false);
}
} else {
startStreaming(parameters, encrypted);
//We just use default video streaming params
VideoStreamingParameters params = (parameters == null) ? new VideoStreamingParameters() : new VideoStreamingParameters(parameters);
DisplayCapabilities dispCap = null;
if (internalInterface.getSystemCapabilityManager() != null) {
dispCap = (DisplayCapabilities) internalInterface.getSystemCapabilityManager().getCapability(SystemCapabilityType.DISPLAY, null, false);
}
if (dispCap != null) {
params.setResolution(dispCap.getScreenParams().getImageResolution());
}
startStreaming(params, encrypted);
}
}

Expand Down
44 changes: 29 additions & 15 deletions base/src/main/java/com/smartdevicelink/streaming/video/VideoStreamingParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,28 @@ public VideoStreamingParameters() {
format = new VideoStreamingFormat();
format.setProtocol(DEFAULT_PROTOCOL);
format.setCodec(DEFAULT_CODEC);
stableFrameRate = true;
}

/**
* deprecated constructor of VideoStreamingParameters. This constructor will be removed in the future version.
* @param displayDensity
* @param frameRate
* @param bitrate
* @param interval
* @param resolution
* @param format
*/
@Deprecated
public VideoStreamingParameters(int displayDensity, int frameRate, int bitrate, int interval,
ImageResolution resolution, VideoStreamingFormat format) {
this.displayDensity = displayDensity;
this.frameRate = frameRate;
this.bitrate = bitrate;
this.interval = interval;
this.resolution = resolution;
this.format = format;
this.stableFrameRate = true;
this.displayDensity = displayDensity;
this.frameRate = frameRate;
this.bitrate = bitrate;
this.interval = interval;
this.resolution = resolution;
this.format = format;
this.stableFrameRate = true;
shiniwat marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -104,14 +114,14 @@ public VideoStreamingParameters(int displayDensity, int frameRate, int bitrate,
* @param stableFrameRate
*/
public VideoStreamingParameters(int displayDensity, int frameRate, int bitrate, int interval,
ImageResolution resolution, VideoStreamingFormat format, boolean stableFrameRate){
this.displayDensity = displayDensity;
this.frameRate = frameRate;
this.bitrate = bitrate;
this.interval = interval;
this.resolution = resolution;
this.format = format;
this.stableFrameRate = stableFrameRate;
ImageResolution resolution, VideoStreamingFormat format, boolean stableFrameRate) {
this.displayDensity = displayDensity;
this.frameRate = frameRate;
this.bitrate = bitrate;
this.interval = interval;
this.resolution = resolution;
this.format = format;
this.stableFrameRate = stableFrameRate;
}

/**
Expand Down Expand Up @@ -171,6 +181,7 @@ public void update(VideoStreamingCapability capability, String vehicleMake) {
this.bitrate = capability.getMaxBitrate() * 1000;
} // NOTE: the unit of maxBitrate in getSystemCapability is kbps.
double scale = DEFAULT_SCALE;
// For resolution and scale, the capability values should be taken rather than parameters specified by developers.
if (capability.getScale() != null) {
scale = capability.getScale();
}
Expand All @@ -197,6 +208,9 @@ public void update(VideoStreamingCapability capability, String vehicleMake) {
// This should be the last call as it will return out once a suitable format is found
final List<VideoStreamingFormat> formats = capability.getSupportedFormats();
if (formats != null && formats.size() > 0) {
if (this.format != null && formats.contains(this.format)) {
return; // given format is supported, so no need to change.
}
for (VideoStreamingFormat format : formats) {
for (VideoStreamingFormat currentlySupportedFormat : currentlySupportedFormats) {
if (currentlySupportedFormat.equals(format)) {
Expand Down