Return an empty slice on safe slice casting error

[tharkum]

The cargo byte-slice-cast crate supports safe u8 -> f32 slice casting through the as_slice_of trait function. However, for dynamic linked crate from rust toolchain (byte-slice-cast-0.2.0) in case if source u8 slice is empty, the WrongAlignment error will be returned, so need to handle this edge case properly and return the empty f32 slice on error.

Please notice that the newest version (byte-slice-cast-1.2.3) is handling this case without alignment error (return an empty slice).

https://docs.rs/byte-slice-cast/0.2.0/src/byte_slice_cast/lib.rs.html#206
https://docs.rs/byte-slice-cast/1.2.3/src/byte_slice_cast/lib.rs.html#263

[tharkum]

There are observable CRASH in the following test:
./mach test-wpt tests/wpt/tests/html/semantics/embedded-content/media-elements/preserves-pitch.html -r

0:03.75 pid:3902622 called `Result::unwrap()` on an `Err` value: WrongAlignment (thread <unnamed>, at /home/XXXXX/workspace/projects/servo/media/backends/gstreamer/player.rs:122)
 0:03.77 pid:3902622    0: servoshell::backtrace::print
 0:03.77 pid:3902622    1: servoshell::panic_hook::panic_hook
 0:03.77 pid:3902622    2: std::panicking::panic_with_hook
 0:03.77 pid:3902622    3: std::panicking::panic_handler::{{closure}}
 0:03.77 pid:3902622    4: std::sys::backtrace::__rust_end_short_backtrace
 0:03.77 pid:3902622    5: __rustc::rust_begin_unwind
 0:03.77 pid:3902622    6: core::panicking::panic_fmt
 0:03.77 pid:3902622    7: core::result::unwrap_failed
 0:03.77 pid:3902622    8: <servo_media_gstreamer::player::GStreamerAudioChunk as core::convert::AsRef<[f32]>>::as_ref
 0:03.77 pid:3902622    9: <servo_media_audio::media_element_source_node::MediaElementSourceNodeRenderer as servo_media_player::audio::AudioRenderer>::render
 0:03.77 pid:3902622   10: servo_media_gstreamer::player::GStreamerPlayer::setup::{{closure}}
 0:03.77 pid:3902622   11: gstreamer_app::app_sink::trampoline_new_sample

[tharkum]

@jdm , @sdroege < Please take a look...

[sdroege]

Why not update from byte-slice-cast 0.2 to 1?

[tharkum]

The byte-slice-cast-1.2.3 could also return the following AlignmentMismatch and LengthMismatch errors (chance is too low, but...), so we must also handle these potential cases by returning an empty slice.

So I decided don't upgrade crate and make it statically linked without significant reason.

[sdroege]

So I decided don't upgrade crate and make it statically linked without significant reason.

Right, but updating seems like a good idea regardless :) There were a couple of bugs fixed after the last couple of years between these two versions.

[sdroege]

I'm not sure this is the right way of handling this. You're going to silently ignore bugs here (it should always be aligned correctly, etc) which are going to be very annoying to debug if you just make this an empty slice.

Also note that backends/gstreamer/audio_decoder.rs has basically the same code but still unwrap()s, so at the very least this is inconsistent.

[sdroege]

And backends/gstreamer/audio_stream_reader.rs converts it into an Err but as above, this should never really happen and if it happens then there is a bug elsewhere that needs to be fixed.

At the very least these cases should be logged.

[sdroege]

byte-slice-cast update is in #469