Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions mp4parse/src/boxes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ box_database!(
H263SpecificBox 0x6432_3633, // "d263"
MP4AudioSampleEntry 0x6d70_3461, // "mp4a"
MP4VideoSampleEntry 0x6d70_3476, // "mp4v"
AMRNBSampleEntry 0x7361_6d72, // "samr" - AMR narrow-band
AMRWBSampleEntry 0x7361_7762, // "sawb" - AMR wide-band
AMRSpecificBox 0x6461_6d72, // "damr"
Comment on lines +140 to +142
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the macro context, it's fine to add these entries without any sort of feature gating, but a comment that these are only used for the 3gpp feature-specific code would be helpful

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll add comment to indicate that these code only used for the 3gpp feature-specific code.
Do I need to file a new PR for this commit?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I need to file a new PR for this commit?

Your PR inspired me to make #279 for use cases like these. Now that it's merged, these entries can be conveniently put behind the feature gate like so:

Suggested change
AMRNBSampleEntry 0x7361_6d72, // "samr" - AMR narrow-band
AMRWBSampleEntry 0x7361_7762, // "sawb" - AMR wide-band
AMRSpecificBox 0x6461_6d72, // "damr"
#[cfg(feature = "3gpp")]
AMRNBSampleEntry 0x7361_6d72, // "samr" - AMR narrow-band
#[cfg(feature = "3gpp")]
AMRWBSampleEntry 0x7361_7762, // "sawb" - AMR wide-band
#[cfg(feature = "3gpp")]
AMRSpecificBox 0x6461_6d72, // "damr"

Please make that change and there's no need for any additional comments. The intent is pretty clear, I think.

You don't need to make a new PR. Please rebase so you can get the changes from #279, then add the other changes I've asked for (in whatever way you like) and update your dreamaniajp:master branch so I can review in this PR.

Copy link
Copy Markdown
Contributor Author

@dreamaniajp dreamaniajp Mar 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll rebase and add the feature gate, thanks!

Update: I saw that you already had the commit about this, thanks!

ESDBox 0x6573_6473, // "esds"
VP8SampleEntry 0x7670_3038, // "vp08"
VP9SampleEntry 0x7670_3039, // "vp09"
Expand Down
29 changes: 29 additions & 0 deletions mp4parse/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,10 @@ pub enum AudioCodecSpecific {
ALACSpecificBox(ALACSpecificBox),
MP3,
LPCM,
AMRSpecificBox(TryVec<u8>),
// Some mp4 file with AMR doesn't have above AMRSpecificBox "damr",
// we use empty box instead.
AMRSpecificEmptyBox,
}

#[derive(Debug)]
Expand Down Expand Up @@ -1113,6 +1117,8 @@ pub enum CodecType {
LPCM, // QT
ALAC,
H263,
AMRNB,
AMRWB,
}

impl Default for CodecType {
Expand Down Expand Up @@ -3742,6 +3748,16 @@ fn read_audio_sample_entry<T: Read>(src: &mut BMFFBox<T>) -> Result<SampleEntry>
let (mut codec_type, mut codec_specific) = match name {
BoxType::MP3AudioSampleEntry => (CodecType::MP3, Some(AudioCodecSpecific::MP3)),
BoxType::LPCMAudioSampleEntry => (CodecType::LPCM, Some(AudioCodecSpecific::LPCM)),
// Some mp4 file with AMR doesn't have AMRSpecificBox "damr" in followed while loop,
// we use empty box by default.
BoxType::AMRNBSampleEntry => (
CodecType::AMRNB,
Some(AudioCodecSpecific::AMRSpecificEmptyBox),
),
BoxType::AMRWBSampleEntry => (
CodecType::AMRWB,
Some(AudioCodecSpecific::AMRSpecificEmptyBox),
),
_ => (CodecType::Unknown, None),
};
let mut protection_info = TryVec::new();
Expand Down Expand Up @@ -3801,6 +3817,19 @@ fn read_audio_sample_entry<T: Read>(src: &mut BMFFBox<T>) -> Result<SampleEntry>
codec_type = CodecType::EncryptedAudio;
protection_info.push(sinf)?;
}
BoxType::AMRSpecificBox => {
if codec_type != CodecType::AMRNB && codec_type != CodecType::AMRWB {
return Err(Error::InvalidData("malformed audio sample entry"));
}
let amr_dec_spec_struc_size = b
.head
.size
.checked_sub(b.head.offset)
.expect("offset invalid");
let amr_dec_spec_struc = read_buf(&mut b.content, amr_dec_spec_struc_size)?;
debug!("{:?} (AMRDecSpecStruc)", amr_dec_spec_struc);
codec_specific = Some(AudioCodecSpecific::AMRSpecificBox(amr_dec_spec_struc));
}
_ => {
debug!("Unsupported audio codec, box {:?} found", b.head.name);
skip_box_content(&mut b)?;
Expand Down
Binary file added mp4parse/tests/amr_nb_1f.3gp
Binary file not shown.
Binary file added mp4parse/tests/amr_wb_1f.3gp
Binary file not shown.
60 changes: 60 additions & 0 deletions mp4parse/tests/public.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ static AVIF_CORRUPT_IMAGES: &str = "tests/corrupt";
// The 1 frame h263 3gp file can be generated by ffmpeg with command
// "ffmpeg -i [input file] -f 3gp -vcodec h263 -vf scale=176x144 -frames:v 1 -an output.3gp"
static VIDEO_H263_3GP: &str = "tests/bbb_sunflower_QCIF_30fps_h263_noaudio_1f.3gp";
// The 1 frame AMR-NB 3gp file can be generated by ffmpeg with command
// "ffmpeg -i [input file] -f 3gp -acodec amr_nb -ar 8000 -ac 1 -frames:a 1 -vn output.3gp"
static AUDIO_AMRNB_3GP: &str = "tests/amr_nb_1f.3gp";
// The 1 frame AMR-WB 3gp file can be generated by ffmpeg with command
// "ffmpeg -i [input file] -f 3gp -acodec amr_wb -ar 16000 -ac 1 -frames:a 1 -vn output.3gp"
static AUDIO_AMRWB_3GP: &str = "tests/amr_wb_1f.3gp";

// Adapted from https://github.com/GuillaumeGomez/audio-video-metadata/blob/9dff40f565af71d5502e03a2e78ae63df95cfd40/src/metadata.rs#L53
#[test]
Expand Down Expand Up @@ -157,6 +163,10 @@ fn public_api() {
mp4::AudioCodecSpecific::LPCM => {
"LPCM"
}
mp4::AudioCodecSpecific::AMRSpecificBox(_)
| mp4::AudioCodecSpecific::AMRSpecificEmptyBox => {
"AMR"
}
},
"ES"
);
Expand Down Expand Up @@ -797,3 +807,53 @@ fn public_video_h263() {
};
}
}

#[test]
fn public_audio_amrnb() {
let mut fd = File::open(AUDIO_AMRNB_3GP).expect("Unknown file");
let mut buf = Vec::new();
fd.read_to_end(&mut buf).expect("File error");

let mut c = Cursor::new(&buf);
let context = mp4::read_mp4(&mut c).expect("read_mp4 failed");
for track in context.tracks {
let stsd = track.stsd.expect("expected an stsd");
let a = match stsd.descriptions.first().expect("expected a SampleEntry") {
mp4::SampleEntry::Audio(ref v) => v,
_ => panic!("expected a AudioSampleEntry"),
};
assert!(a.codec_type == mp4::CodecType::AMRNB);
let _codec_specific = match a.codec_specific {
mp4::AudioCodecSpecific::AMRSpecificBox(_)
| mp4::AudioCodecSpecific::AMRSpecificEmptyBox => true,
_ => {
panic!("expected a AMRSpecificBox",);
}
};
}
}

#[test]
fn public_audio_amrwb() {
let mut fd = File::open(AUDIO_AMRWB_3GP).expect("Unknown file");
let mut buf = Vec::new();
fd.read_to_end(&mut buf).expect("File error");

let mut c = Cursor::new(&buf);
let context = mp4::read_mp4(&mut c).expect("read_mp4 failed");
for track in context.tracks {
let stsd = track.stsd.expect("expected an stsd");
let a = match stsd.descriptions.first().expect("expected a SampleEntry") {
mp4::SampleEntry::Audio(ref v) => v,
_ => panic!("expected a AudioSampleEntry"),
};
assert!(a.codec_type == mp4::CodecType::AMRWB);
let _codec_specific = match a.codec_specific {
mp4::AudioCodecSpecific::AMRSpecificBox(_)
| mp4::AudioCodecSpecific::AMRSpecificEmptyBox => true,
_ => {
panic!("expected a AMRSpecificBox",);
}
};
}
}
15 changes: 14 additions & 1 deletion mp4parse_capi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ pub enum Mp4parseCodec {
Ec3,
Alac,
H263,
AMRNB,
AMRWB,
}

impl Default for Mp4parseCodec {
Expand Down Expand Up @@ -895,6 +897,14 @@ fn get_track_audio_info(
}
AudioCodecSpecific::MP3 => Mp4parseCodec::Mp3,
AudioCodecSpecific::ALACSpecificBox(_) => Mp4parseCodec::Alac,
AudioCodecSpecific::AMRSpecificBox(_) | AudioCodecSpecific::AMRSpecificEmptyBox
if audio.codec_type == CodecType::AMRNB =>
{
Mp4parseCodec::AMRNB
}
AudioCodecSpecific::AMRSpecificBox(_) | AudioCodecSpecific::AMRSpecificEmptyBox => {
Mp4parseCodec::AMRWB
}
};
sample_info.channels = audio.channelcount as u16;
sample_info.bit_depth = audio.samplesize;
Expand Down Expand Up @@ -955,7 +965,10 @@ fn get_track_audio_info(
sample_info.codec_specific_config.length = alac.data.len() as u32;
sample_info.codec_specific_config.data = alac.data.as_ptr();
}
AudioCodecSpecific::MP3 | AudioCodecSpecific::LPCM => (),
AudioCodecSpecific::MP3
| AudioCodecSpecific::LPCM
| AudioCodecSpecific::AMRSpecificBox(_)
| AudioCodecSpecific::AMRSpecificEmptyBox => (),
}

if let Some(p) = audio
Expand Down