-
-
Notifications
You must be signed in to change notification settings - Fork 130
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
Fix all warnings. #162
Fix all warnings. #162
Changes from 8 commits
b3643d6
2c77cdd
473cea1
4fb7b04
92fc127
65de24a
e40ef80
13c73c4
c781bb7
d0ef039
96f2f5c
b778fb0
42dc06e
b5e2252
e375c4c
765c610
64b109d
255ce76
4c8dc7d
2b11885
f1f66c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,7 +128,7 @@ let create_decoder input = | |
processed := !processed + Array.length data.(0) | ||
with _ -> () end; | ||
drop pos ; | ||
let content,length = | ||
let content,_ = | ||
resampler ~audio_src_rate:(float sample_freq) data | ||
in | ||
(* TODO assert (Array.length content.(0) = length) ? *) | ||
|
@@ -179,7 +179,7 @@ let () = | |
"AAC" | ||
~sdoc:"Use libfaad to decode AAC if MIME type or file extension \ | ||
is appropriate." | ||
(fun ~metadata filename kind -> | ||
(fun ~metadata:_ filename kind -> | ||
(* Before doing anything, check that we are allowed to produce | ||
* audio, and don't have to produce midi or video. Only then | ||
* check that the file seems relevant for AAC decoding. *) | ||
|
@@ -242,7 +242,7 @@ struct | |
let mp4 = Faad.Mp4.openfile ?seek:input.Decoder.lseek read in | ||
let resampler = Rutils.create_audio () in | ||
let track = Faad.Mp4.find_aac_track mp4 in | ||
let sample_freq, chans = Faad.Mp4.init mp4 dec track in | ||
let sample_freq, _ = Faad.Mp4.init mp4 dec track in | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we check that we get the right number of chans or somehow use this information? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, after reading the code, it seems to me that the generator, which is fed with audio data, has some smarts to accept various audio content according to the frame type. Thus, I suspect that it's acceptable for the generator in some cases, e.g. audio of type @dbaelde should be able to confirm if I'm on crack or what.. ^^ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi guys. I did a quick check, and it seems that we don't need to check the number of channels here. The resampler (from Rutils) supports varying number of channels, and in the end the decoder (created using Decoder.Buffered.file_decoder) makes sure that the number of channels actually does not change. |
||
let nb_samples = Faad.Mp4.samples mp4 track in | ||
let sample = ref 0 in | ||
let pos = ref 0 in | ||
|
@@ -254,7 +254,7 @@ struct | |
begin try | ||
pos := !pos + (Array.length data.(0)) | ||
with _ -> () end; | ||
let content,length = | ||
let content,_ = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||
resampler ~audio_src_rate:(float sample_freq) data | ||
in | ||
Generator.set_mode gen `Audio; | ||
|
@@ -316,7 +316,7 @@ let () = | |
"MP4" | ||
~sdoc:"Use libfaad to decode MP4 if MIME type or file extension \ | ||
is appropriate." | ||
(fun ~metadata filename kind -> | ||
(fun ~metadata:_ filename kind -> | ||
(* Before doing anything, check that we are allowed to produce | ||
* audio, and don't have to produce midi or video. Only then | ||
* check that the file seems relevant for MP4 decoding. *) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,7 +67,7 @@ let file_deco filename = | |
let fill frame = | ||
let pos = Frame.position frame in | ||
let duration = Frame.seconds_of_master (size-pos) in | ||
let rec aux t' = | ||
let rec aux () = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is weird.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think you need to keep this warning (because it says something useful: this piece of code is unfinished) or do some more modification. It may be okay to get rid of t' because the reference to t is used to keep track of time. I don't know why there is a reference, maybe to use it at the end after aux has returned.. but right now, the last update is useless because the ref is forgotten right after. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know what last update you have in mind but after re-reading this code, it looks fine to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked at the code again. It didn't make sense at first, but now it does, and I believe that my previous comment was misleaded. The reference t keeps track of the time in the metadata stream, and lives through all calls of fill. I don't see any problem with this code. Thanks! |
||
match !events with | ||
| (ts,k,v)::tl -> | ||
if ts < !t+.duration then | ||
|
@@ -76,13 +76,13 @@ let file_deco filename = | |
Hashtbl.add meta k v ; | ||
Frame.set_metadata frame pos meta ; | ||
events := tl ; | ||
aux pos | ||
aux () | ||
else | ||
Frame.add_break frame size | ||
| [] -> Frame.add_break frame pos | ||
in | ||
ignore (Frame.content_of_type frame pos empty) ; | ||
aux pos ; | ||
aux () ; | ||
t := !t +. Frame.seconds_of_master (Frame.position frame - pos) ; | ||
-1 (* TODO remaining time *) | ||
in | ||
|
@@ -93,7 +93,7 @@ let file_deco filename = | |
|
||
let () = | ||
Decoder.file_decoders#register "META" | ||
(fun ~metadata filename kind -> | ||
(fun ~metadata:_ filename kind -> | ||
if Frame.type_has_kind empty kind then begin | ||
ignore (parse_file filename) ; | ||
Some (fun () -> file_deco filename) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why
resampler
returns a length? It seems to me that it is the length of the first channel always so it can be computed if necessary (and I doubt it will be the case...)