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

Implement ID3 Timed Metadata support for HLS #67

Closed
andudo opened this issue Oct 3, 2014 · 15 comments
Closed

Implement ID3 Timed Metadata support for HLS #67

andudo opened this issue Oct 3, 2014 · 15 comments
Assignees

Comments

@andudo
Copy link
Contributor

andudo commented Oct 3, 2014

No description provided.

@bhnath
Copy link

bhnath commented Oct 7, 2014

Hey there,

I was previously implementing this using a fork of the ExoPlayer and martinbonnin's HLS branch (https://github.com/martinbonnin/ExoPlayer/tree/hls). I am currently working on adapting my ID3 work into this hls branch. Would that be a helpful addition for a pull request? Or will you guys be taking care of it on your own?

@ojw28
Copy link
Contributor

ojw28 commented Oct 7, 2014

We'll be adding support directly (very soon). There will be a point where external contributions will become really helpful, but I don't think we're quite there yet.

@TheHal85
Copy link

I had some issues with TimedMetadata with this implementation, and had to make some minor changes to get the results I anticipated. Granted, I am not an expert in the matter, please see the note I made in the commit for this issue d3a05c9

@andudo
Copy link
Contributor Author

andudo commented Oct 29, 2014

@TheHal85 thanks for your feedback! We did it this way cause we mainly interested in TXXX frames, and TXXX frame by itself contains two strings (Description and Value). But it sounds like you're right the frame type should be the key, I'll investigate it more and update the code.

@TheHal85
Copy link

No worries, and thank you for the implementation. I had my eye on this issue and wanted to give it a shot with one of our HLS streams and I was always getting an empty array of metadata. I wasn't sure if the issue was our implementation of metadata insertion or not. Your implementation makes sense if you were concentrating on TXXX frames though. Thanks again!

@andudo
Copy link
Contributor Author

andudo commented Nov 5, 2014

@TheHal85 please take a look at the updates I've made based on your suggestion 71f918c

@TheHal85
Copy link

TheHal85 commented Nov 5, 2014

Works for me @andudo. Just in case anyone else uses the implementation, below is the onMetadata I used for testing it out. We are currently throwing closed captions into an ID3 tag, which hopefully is a temporary solution.

    @Override
    public void onMetadata(Map<String, Object> metadata) {
        for (Object o : metadata.entrySet()) {
            Map.Entry pairs = (Map.Entry) o;
            if (metadata.containsKey(TxxxMetadata.TYPE)) {
                TxxxMetadata txxxMetadata = (TxxxMetadata) metadata.get(TxxxMetadata.TYPE);
                Log.i(TAG, String.format("ID3 TimedMetadata: description=%s, value=%s",
                        txxxMetadata.description, txxxMetadata.value));
            } else {
                Log.d(TAG, String.format("ID3 TimedMetadata: key=%s, value=%s",pairs.getKey(),new String((byte[]) pairs.getValue())));
                if (metadata.containsKey("TIT1")) {
                    String closedCaptioning = new String((byte[]) metadata.get("TIT1"));
                    //bad to call override onText for TextListener, but works for now
                    onText(closedCaptioning);
                }
            }
        }
    }

Ninja Edit -
Also - I am making the assumption that the Frame is one of the Text based Frames. Instead of the

else {
Log.d(TAG, String.format("ID3 TimedMetadata: key=%s, value=%s",pairs.getKey(),new String((byte[]) pairs.getValue())));

it'd be better to see if the frame key started with "T" but I didn't have the time to really implement that.

@andudo
Copy link
Contributor Author

andudo commented Nov 5, 2014

Just an FYI, @TheHal85 your code snippet is a little buggy, probably a copy&paste issue. For example, if there is a TXXX frame in the map and two others, it will just log the same TXXX 3 times. Use the map entry if you're iterating through it and don't call get() or containsKey() on it. Also cast the element to the right type in the for loop for (Map.Entry<String, Object> entry : metadata.entrySet()) {...}.

We're working on adding Closed Captions support, so hopefully you won't need to do those tricks soon.

@TheHal85
Copy link

TheHal85 commented Nov 5, 2014

Thanks @andudo. This was definitely a quick hack just to be see if it worked out for us, didn't go through all the scenarios. Thanks for the info though, it's appreciated.

@andudo andudo closed this as completed Nov 5, 2014
@perchrh
Copy link

perchrh commented Nov 6, 2014

I came across an issue with dev-hls current, where ExoPlayer finds the track of type "application/id3" from the stream, but fails reading the track.
MetadataTrackRenderer.java:139 never receives SampleSource#SAMPLE_READ from the source, it's always NOTHING_READ. As a result, the subtitles are not displayed.
In HlsSampleSource's read method none of the if-statements match and it returns from the final line of the method.

Test stream that has this issue, EAC-608 format:
http://now.video.nfl.com/i/captiontest/closedcaptiontest_,350k,550k,.mp4.csmil/master.m3u8
It plays with subtitles in Quicktime.

@andudo
Copy link
Contributor Author

andudo commented Nov 6, 2014

@perchrh ID3 and Closed Captions are two different separate things. What @TheHal85 is doing is a hacky work around while we don't have Closed Captions support.
Chunks in your stream have ID3 track registered in PMT, but no packets with ID3 data ever appear, thats why you see that MetadataTrackRenderer never gets SAMPLE_READ.
But you stream does have Closed Captions, which are stored as Closed Captions, not as ID3. Thats why Quicktime displays them. ExoPlayer doesn't support Closed Captions yet, thats why you don't see them displayed in ExoPlayer. There is another open issue for it #68

@perchrh
Copy link

perchrh commented Nov 7, 2014

@andudo Thanks for explaining it to me. I erroneously thought every type of subtitle would be present in the ID3 container.

@outlying
Copy link

outlying commented Feb 4, 2016

Current implementation doesn't seem valid. According to ID3v2.3.0 documentation

There may be more than one "TXXX" frame in each tag

but since Map is used to build metadata information in Id3Parser we only get information about last TXXX frame read from ID3 tag.

It's more like ID3 parsing problem than Timed Matadata, but still TM is not working because of that

@jlacivita
Copy link

@outlying I suggest filing a bug, not commenting on a closed enhancement

@outlying
Copy link

@jlacivita I created PR after this comment #1234

@google google locked and limited conversation to collaborators Jun 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants