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

Create VLCMedia Tests #19

Closed
wants to merge 2 commits into from
Closed

Create VLCMedia Tests #19

wants to merge 2 commits into from

Conversation

@mkchoi212
Copy link
Contributor

@mkchoi212 mkchoi212 commented Jun 19, 2018

Commits need to be rebased once #18 is merged.

@mkchoi212 mkchoi212 force-pushed the mkchoi212:media branch 2 times, most recently from e6820b5 to d255d39 Jun 19, 2018
return [NSString stringWithUTF8String:ret];

return @"";
return [NSString stringWithUTF8String:ret];

This comment has been minimized.

@carolanitz

carolanitz Jun 25, 2018
Member

this will crash with ret = nil. This was not redundant line

This comment has been minimized.

@mkchoi212

mkchoi212 Jun 28, 2018
Author Contributor

I tracked it down and seems like *ret = libvlc_media_get_codec_description(...) calls vlc_fourcc_GetDescription, which has the following return statement

return LookupCat(fourcc, &ret, cat) ? ret : "";

Doesn't this mean that vlc_fourcc_GetDescription will never return nil?

@fkuehne fkuehne self-requested a review Jun 26, 2018
@fkuehne
Copy link
Contributor

@fkuehne fkuehne commented Jun 26, 2018

After removing the commit NACKed by Caro, this is good to go for me and I like it :)

@mkchoi212 mkchoi212 force-pushed the mkchoi212:media branch 3 times, most recently from 1286fa7 to 55921e1 Jun 27, 2018
@mkchoi212 mkchoi212 force-pushed the mkchoi212:media branch from 55921e1 to 47cfd4b Jul 5, 2018
@mkchoi212 mkchoi212 force-pushed the mkchoi212:media branch from 47cfd4b to acecf75 Jul 5, 2018
@mkchoi212
Copy link
Contributor Author

@mkchoi212 mkchoi212 commented Jul 9, 2018

Ping 🔔

@carolanitz
Copy link
Member

@carolanitz carolanitz commented Jul 10, 2018

merged with faf2a0d

@carolanitz carolanitz closed this Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants