955981 - MediaEncoder - Make InterleaveTrackData function can be shared

Status: RESOLVED FIXED

(Core :: Audio/Video: Recording, defect)

Description

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

We have a implementation on AudioTrackEncoder::InterleaveTrackData
But it only can be used when class inherit from AudioTrackEncoder.
In OmxWrapper we also need this but it don't inherit from that interface.

Comment 1

[Shelly Lin [:shelly]]

Take it as my 2014 first bug lol.

Comment 2

[Shelly Lin [:shelly]]

Attachment: Make function InterleaveTrackData public and static

This is mostly for the purpose of preventing duplicate copy in bug 879668, OMXCodecWrapper.cpp, ln:358.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8356428 [details] [diff] [review]
Make function InterleaveTrackData public and static

Review of attachment 8356428 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/encoder/OpusTrackEncoder.cpp
@@ +281,5 @@
>  
>      if (!chunk.IsNull()) {
>        // Append the interleaved data to the end of pcm buffer.
> +      AudioTrackEncoder::InterleaveTrackData(chunk, frameToCopy, mChannels,
> +                         pcm.Elements() + frameCopied * mChannels);

Indent this line by just two spaces to the right of the previous line.

Comment 4

[Shelly Lin [:shelly]]

Attachment: Make function InterleaveTrackData public and static (checkin-patch)

Updated with nit fix and carry r+ from roc.

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=736af568b86e

Hi John,
Could you update the |OMXCodecWrapper| once this is checked-in? Thanks a lot.

Comment 5

[John Lin [:jhlin][:jolin]]

(In reply to Shelly Lin [:shelly] from comment #4)
> Created attachment 8356500 [details] [diff] [review]
> Make function InterleaveTrackData public and static (checkin-patch)
> 
> Updated with nit fix and carry r+ from roc.
> 
> Try result:
> https://tbpl.mozilla.org/?tree=Try&rev=736af568b86e
> 
> Hi John,
> Could you update the |OMXCodecWrapper| once this is checked-in? Thanks a lot.

 Sure. I will update the patch and ask for roc's help to review it. Thanks a lot for your help! <(__)>

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/7d434e24be3b

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7d434e24be3b