1275812 - experiment with propagating codec

Status: RESOLVED DUPLICATE of bug 1267887

(Core :: Audio/Video: Playback, defect, P2)

Description

[Ralph Giles (:rillian)]

We need to make the track codec available to the caller. In lieu of a good way to share rust strings with C++ use an enum.

Comment 1

[Ralph Giles (:rillian)]

Attachment: MozReview Request: Bug 1275812 - Experiment with exporting track mime-type. r?kinetik

Review commit: https://reviewboard.mozilla.org/r/55338/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55338/

Comment 2

[Ralph Giles (:rillian)]

we need a better fix for this upstream, but this part has been pretty stable in my development tree and does no harm, so I'd like to get it landed.

Comment 3

[Matthew Gregan [:kinetik]]

Comment on attachment 8756698 [details]
MozReview Request: Bug 1275812 - Experiment with exporting track mime-type. r?kinetik

https://reviewboard.mozilla.org/r/55338/#review52092

::: media/libstagefright/binding/mp4parse/capi.rs:177
(Diff revision 1)
>          TrackType::Video => TRACK_TYPE_H264,
>          TrackType::Audio => TRACK_TYPE_AAC,
>          TrackType::Unknown => return MP4PARSE_ERROR_UNSUPPORTED,
>      };
>  
> +    info.codec = match &*context.tracks[track_index].mime_type {

&context.tracks[track_index].mime_type[..] might be slightly more idiomatic?

Comment 4

[Ralph Giles (:rillian)]

(In reply to Matthew Gregan [:kinetik] from comment #3)

> > +    info.codec = match &*context.tracks[track_index].mime_type {
> 
> &context.tracks[track_index].mime_type[..] might be slightly more idiomatic?

I couldn't get that to work. the &* is needed to trigger the Deref trait's conversion from &String to &str.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa1d245e0af3

Comment 6

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/856903d82b90

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/de91db2be204

Comment 8

[Ralph Giles (:rillian)]

From IRC: We had crashes on win32 debug with the initial inbound check-in. Which didn't go away after the backout. Trying again with a CLOBBER.

Comment 9

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8aa94e6a0c5c

Comment 10

[Matthew Gregan [:kinetik]]

(In reply to Ralph Giles (:rillian) needinfo me from comment #4)
> (In reply to Matthew Gregan [:kinetik] from comment #3)
> 
> > > +    info.codec = match &*context.tracks[track_index].mime_type {
> > 
> > &context.tracks[track_index].mime_type[..] might be slightly more idiomatic?
> 
> I couldn't get that to work. the &* is needed to trigger the Deref trait's
> conversion from &String to &str.

&* is fine anyway, I couldn't remember and dug around afterwards to check.  I dropped the comment in MozReview but I guess it was already published to Bugzilla.

Comment 11

[Ralph Giles (:rillian)]

I cannot reproduce the crashes on a local win7 build.

Comment 12

[Ralph Giles (:rillian)]

Fresh try push on today's m-c https://treeherder.mozilla.org/#/jobs?repo=try&revision=098731d2ad9b

Comment 13

[Ralph Giles (:rillian)]

This is now working as part of the changes on the original bug 1267887.