Implementing a very simple dummy self-threaded decoder

I'm creating an app that will use ffmpeg decoder, so I created a dummy decoder to test my design choices. It's supposed to have its own thread and then I can send encoded video through on_consume and produce decoded packets through on_produce, which are callbacks passed to it on construction. On Drop, it should wait for the thread to finish using join. It'll always finish fast because the decoder loop executes at least 30 times per second, giving a chance for the should_continue to be setted false and thus breaking the loop.

I'd like to know what you guys think about this design and if the choices for my smart pointers are ok. I didn't paste the traits because they're self explanatory through the trait implementation

    use super::decoded_packet::DecodedPacket;
    use super::decoder::{Codec, Decoder};
    use super::defaults;
    use super::encoded_packet::EncodedPacket;
    use super::stoppable::Stoppable;
    use std::sync::atomic::{AtomicBool, Ordering};
    use std::sync::Arc;
    use std::thread::{sleep, spawn, JoinHandle};

    pub struct DummyDecoder {
        pub on_consume: Arc<dyn Fn() -> Option<EncodedPacket> + Send + Sync + 'static>,
        pub on_produce: Arc<dyn Fn(DecodedPacket) + Send + Sync + 'static>,
        should_continue: Arc<AtomicBool>,
        run_thread: Option<JoinHandle<()>>,
    }

    impl DummyDecoder {
        pub fn new(
            on_consume: Arc<dyn Fn() -> Option<EncodedPacket> + Send + Sync + 'static>,
            on_produce: Arc<dyn Fn(DecodedPacket) + Send + Sync + 'static>,
        ) -> DummyDecoder {
            DummyDecoder {
                on_consume: on_consume,
                on_produce: on_produce,
                should_continue: Arc::new(AtomicBool::new(true)),
                run_thread: None,
            }
        }
    }

    impl Decoder for DummyDecoder {
        fn set_on_consume(
            &mut self,
            f: Arc<dyn Fn() -> Option<EncodedPacket> + Send + Sync + 'static>,
        ) {
            self.on_consume = f;
        }
        fn set_on_produce(&mut self, f: Arc<dyn Fn(DecodedPacket) + Send + Sync + 'static>) {
            self.on_produce = f;
        }
        fn codec(&self) -> Codec {
            Codec::H264
        }
    }

    impl Stoppable for DummyDecoder {
        fn should_continue(&self) -> bool {
            self.should_continue.load(Ordering::Relaxed)
        }

        fn run(&mut self) {
            let on_consume_ = self.on_consume.clone();
            let on_produce_ = self.on_produce.clone();
            let should_continue_ = self.should_continue.clone();
            self.run_thread = Some(spawn(move || {
                while should_continue_.load(Ordering::Relaxed) {
                    let encoded_packet = (on_consume_)();
                    //Simulate transform of encoded_packet in decoded_packet here
                    sleep(defaults::default_timeout_stoppable);
                    let decoded_packet = DecodedPacket { data: Vec::new() };
                    (on_produce_)(decoded_packet);
                }
            }));
        }
        fn stop(&self) {
            self.should_continue.store(false, Ordering::Relaxed);
        }
    }

    impl Drop for DummyDecoder {
        fn drop(&mut self) {
            self.stop();
            self.run_thread.take().unwrap().join();
        }
    }

I used Arc for the callbacks (on_produce and on_consume) because the outside thread will use them to send the input data from an RTSP client and consume the output data in an OpenGL renderer, so I think it makes sense to make this choice.

I couldn't use fn should_continue(&self) inside fn run because I needed to clone the should_continue: Arc<AtomicBool> so it can be moved to the inside of the closure. Is there a solution for this? Or I should do the repeating process of while should_continue_.load(Ordering::Relaxed)? I think I might need to remove fn should_continue()-> bool from Stoppable.

You might want to change the category to "code review" if that is more appropriate. Some people monitor only specific categories, so might not bother with the "help" category unless they have a lot of time.

I'd recommend using type aliases:

type OnConsumeFn = dyn Fn() -> Option<EncodedPacket> + Send + Sync + 'static;
type OnProduceFn = dyn Fn(DecodedPacket) + Send + Sync + 'static;

IMO, type aliases make the code more readable and maintainable. If you declare them pub, they can be used by your lib users and they appear in the docs, as well, otherwise the docs will show the aliased type and users will have to declare the full type or their own type alias.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.