Conversation
There was a problem hiding this comment.
Pull request overview
Updates AIDL interface documentation for the audio/video decoder HALs to clarify post-signalEOS() behaviour and how EOS is indicated via FrameMetadata.endOfStream, aligning the contract discussed in gh #358 with the interface comments.
Changes:
- Clarify
decodeBuffer()return behaviour whensignalEOS()has already been called. - Document that
decodeBuffer()calls aftersignalEOS()(until flush/restart) must returnfalse. - Refine
FrameMetadata.endOfStreamwording to describe EOS signalling on the final output callback.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
videodecoder/current/com/rdk/hal/videodecoder/IVideoDecoderController.aidl |
Updates decodeBuffer() / signalEOS() documentation to specify false after EOS until flush/restart. |
videodecoder/current/com/rdk/hal/videodecoder/FrameMetadata.aidl |
Clarifies meaning/timing of endOfStream for video decoder output. |
audiodecoder/current/com/rdk/hal/audiodecoder/IAudioDecoderController.aidl |
Mirrors the video decoder documentation updates for audio decoder EOS behaviour. |
audiodecoder/current/com/rdk/hal/audiodecoder/FrameMetadata.aidl |
Clarifies meaning/timing of endOfStream for audio decoder output (plus whitespace clean-up). |
videodecoder/current/com/rdk/hal/videodecoder/IVideoDecoderController.aidl
Outdated
Show resolved
Hide resolved
audiodecoder/current/com/rdk/hal/audiodecoder/IAudioDecoderController.aidl
Outdated
Show resolved
Hide resolved
videodecoder/current/com/rdk/hal/videodecoder/FrameMetadata.aidl
Outdated
Show resolved
Hide resolved
audiodecoder/current/com/rdk/hal/audiodecoder/FrameMetadata.aidl
Outdated
Show resolved
Hide resolved
sensible changes Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates AIDL documentation to clarify end-of-stream (EOS) semantics for the audio/video decoder controllers and their FrameMetadata.endOfStream output flag, aligning expected decodeBuffer() behaviour after signalEOS().
Changes:
- Document that
decodeBuffer()returnsfalseif called aftersignalEOS()until the decoder is flushed or restarted. - Clarify that
FrameMetadata.endOfStreamis an output flag set on the final frame callback after EOS, once all queued frames are output. - Minor whitespace cleanup in audio
FrameMetadata.aidl.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
videodecoder/current/com/rdk/hal/videodecoder/IVideoDecoderController.aidl |
Expands decodeBuffer()/signalEOS() documentation to specify post-EOS decodeBuffer() returns false. |
videodecoder/current/com/rdk/hal/videodecoder/FrameMetadata.aidl |
Clarifies meaning/timing of endOfStream flag in decoder output metadata. |
audiodecoder/current/com/rdk/hal/audiodecoder/IAudioDecoderController.aidl |
Mirrors video controller doc updates for post-EOS decodeBuffer() behaviour. |
audiodecoder/current/com/rdk/hal/audiodecoder/FrameMetadata.aidl |
Clarifies meaning/timing of endOfStream flag and removes trailing whitespace. |
You can also share your feedback on Copilot code review. Take the survey.
bhanucbp
left a comment
There was a problem hiding this comment.
address review comments
| * End of stream indicator. | ||
| * End of stream flag for decoder output. | ||
| * Set to true on the final frame output callback after signalEOS(), once all queued frames have been output. | ||
| * This may correspond to an end of stream marker in the audio bitstream, when present. |
There was a problem hiding this comment.
audio bitstreams will not have eos markers present
| * End of stream indicator. | ||
| * End of stream flag for decoder output. | ||
| * Set to true on the final frame output callback after signalEOS(), once all queued frames have been output. | ||
| * This may correspond to an end of stream marker in the audio bitstream, when present. |
There was a problem hiding this comment.
there is dummy onFrameOutput call with -1 pts and null framehandle after signalEOS(). This info missing, add this info to onFrameOutput
There was a problem hiding this comment.
Is this SOC specific implementation ?
There was a problem hiding this comment.
No, this is not implementation-specific. The callback behavior should be explicitly documented; otherwise, the caller will not be aware of the final dummy frame.
| /** | ||
| * End of stream marker found in the video bitstream. | ||
| * End of stream flag for decoder output. | ||
| * Set to true on the final frame output callback after signalEOS(), once all queued frames have been output. |
There was a problem hiding this comment.
its not on the final frame, there will be a dummy onFrameOutput with -1 pts and null frame handle after final frame delivered
There was a problem hiding this comment.
Is this implementation specific ?
There was a problem hiding this comment.
No, this is not implementation-specific. The behavior of the callbacks needs to be clearly defined; otherwise, the caller may not know that a final dummy frame (with invalid PTS and null handle) is delivered after signalEOS().
There was a problem hiding this comment.
Pull request overview
Updates the Audio/Video decoder AIDL interface documentation to clarify end-of-stream (EOS) behaviour, including how EOS affects subsequent buffer submission and how EOS is communicated in frame metadata.
Changes:
- Documented that
decodeBuffer()must returnfalseaftersignalEOS()for both audio and video decoders. - Expanded
FrameMetadata.endOfStreamdocumentation to describe EOS signalling scenarios (including “EOS-only” callbacks). - Minor whitespace cleanup in audio
FrameMetadata.aidl.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| videodecoder/current/com/rdk/hal/videodecoder/IVideoDecoderController.aidl | Adds post-signalEOS() decodeBuffer() behaviour note. |
| videodecoder/current/com/rdk/hal/videodecoder/FrameMetadata.aidl | Expands endOfStream semantics for video output callbacks. |
| audiodecoder/current/com/rdk/hal/audiodecoder/IAudioDecoderController.aidl | Adds post-signalEOS() decodeBuffer() behaviour note. |
| audiodecoder/current/com/rdk/hal/audiodecoder/FrameMetadata.aidl | Expands endOfStream semantics for audio output callbacks and removes trailing whitespace. |
No description provided.