ParadoxSpiral / libmpv-rs

A libmpv abstraction written in rust that's easy to use and provides the ability to read next to all video and audio codecs.
GNU Lesser General Public License v2.1
99 stars 35 forks source link

change `Error::Loadfiles::error` type from `Rc` to `Box` #36

Open m4rch3n1ng opened 8 months ago

m4rch3n1ng commented 8 months ago

change the type for Error::Loadfiles { error } from an Rc to a Box

context

i'm writing a music player using this library to play music via mpv

/// wrapper struct for mpv
pub struct Player(Mpv);

and i want to create a wrapper error enum and pull the inner error out of libmpv::Error::Loadfiles.

#[derive(Debug, thiserror::Error)]
pub enum PlayerError {
    #[error("invalid utf8")]
    InvalidUtf8,
    #[error("null error")]
    Null,
    // ...
}

but when want to impl From<libmpv::Error> for the enum, i have to do some annoying Rc fuckery to get at the inner value, even though the Rc is only created once and never cloned

impl From<libmpv::Error> for PlayerError {
    fn from(value: libmpv::Error) -> Self {
        match value {
            libmpv::Error::Loadfiles { index: _, error } => match Rc::into_inner(error) {
                Some(error) => PlayerError::from(error),
                // why?
                None => unreachable!()
            },
            // ...
        }
    }
}

if you were to change the type into a Box everything works the same, but actually using the error type is a lot less annyoing to work with

impl From<libmpv::Error> for PlayerError {
    fn from(value: libmpv::Error) -> Self {
        match value {
            libmpv::Error::Loadfiles { index: _, error } => PlayerError::from(*error),
            // ...
        }
    }
}

i couldn't find a reason for why the error type uses an Rc instead of a Box, but the Rc was added in commit 53b430d after a comment, that was added in commit 9043418


as an aside, the Rc prevents the error to be used with the anyhow or color_eyre crate, as the anyhow::Error result has type bounds for Send and Sync

impl Player {
    pub fn new() -> anyhow::Result<Self> { // same with color_eyre::Result
        let mpv = Mpv::new()?; // `std::rc::Rc<libmpv::Error>` cannot be sent between threads safely
        mpv.set_property("vo", "null")?; // `std::rc::Rc<libmpv::Error>` cannot be sent between threads safely

        let player = Player(mpv);
        Ok(player)
    }
}