Count power state requests in device, not sensor

Author: OhadMeir

src/linux/backend-v4l2.cpp

@@ -1205,7 +1205,8 @@ namespace librealsense
         }
 
         v4l_uvc_device::v4l_uvc_device(const uvc_device_info& info, bool use_memory_map)
-            : _name(info.id), 
+            : _power_counter( 0 ),
+              _name(info.id), 
               _device_path(info.device_path),
               _device_usb_spec(info.conn_spec),
               _info(info),
@@ -1814,15 +1815,44 @@ namespace librealsense
 
         void v4l_uvc_device::set_power_state(power_state state)
         {
-            if (state == D0 && _state == D3)
+            std::lock_guard< std::recursive_mutex > lock( _power_lock );
+            switch (state)
             {
-                map_device_descriptor();
+            case D0:
+            {
+                if( _power_counter.fetch_add( 1 ) == 0 )
+                {
+                    try
+                    {
+                        map_device_descriptor();
+                    }
+                    //In case of failure need to decrease use counter
+                    catch( std::exception const & e )
+                    {
+                        _power_counter.fetch_add( -1 );
+                        throw e;
+                    }
+                    catch( ... )
+                    {
+                        _power_counter.fetch_add( -1 );
+                        throw;
+                    }
+                }
+                break;
             }
-            if (state == D3 && _state == D0)
+            case D3:
             {
-                close(_profile);
-                unmap_device_descriptor();
+                if( _power_counter.fetch_add( -1 ) == 1 )
+                {
+                    close(_profile);
+                    unmap_device_descriptor();
+                }
+                break;
+            }
+            default:
+                throw std::runtime_error("illegal power state request");
             }
+
             _state = state;
         }
 

src/linux/backend-v4l2.h

@@ -407,6 +407,8 @@ namespace librealsense
 
             static bool get_devname_from_video_path(const std::string& real_path, std::string& devname);
 
+            std::recursive_mutex _power_lock;
+            std::atomic< int > _power_counter;
             power_state _state = D3;
             std::string _name = "";
             std::string _device_path = "";

src/mf/mf-uvc.cpp

@@ -773,23 +773,55 @@ namespace librealsense
 
         void wmf_uvc_device::set_power_state(power_state state)
         {
-            if (state == _power_state)
-                return;
+            std::lock_guard< std::recursive_mutex > lock( _source_lock );
 
             switch (state)
             {
-            case D0: set_d0(); break;
-            case D3: set_d3(); break;
+            case D0:
+            {
+                if( _power_counter.fetch_add( 1 ) == 0 )
+                {
+                    try
+                    {
+                        set_d0();
+                    }
+                    //In case of failure need to decrease use counter
+                    catch( std::exception const & e )
+                    {
+                        _power_counter.fetch_add( -1 );
+                        throw e;
+                    }
+                    catch( ... )
+                    {
+                        _power_counter.fetch_add( -1 );
+                        throw;
+                    }
+                }
+                break;
+            }
+            case D3:
+            {
+                if( _power_counter.fetch_add( -1 ) == 1 )
+                {
+                    set_d3();
+                }
+                break;
+            }
             default:
                 throw std::runtime_error("illegal power state request");
             }
         }
 
-        wmf_uvc_device::wmf_uvc_device(const uvc_device_info& info,
-            std::shared_ptr<const wmf_backend> backend)
-            : _streamIndex(MAX_PINS), _info(info), _is_flushed(), _has_started(), _backend(std::move(backend)),
-            _systemwide_lock(info.unique_id.c_str(), WAIT_FOR_MUTEX_TIME_OUT),
-            _location(""), _device_usb_spec(usb3_type)
+        wmf_uvc_device::wmf_uvc_device( const uvc_device_info & info, std::shared_ptr< const wmf_backend > backend )
+            : _streamIndex( MAX_PINS )
+            , _info( info )
+            , _is_flushed()
+            , _has_started()
+            , _backend( std::move( backend ) )
+            , _systemwide_lock( info.unique_id.c_str(), WAIT_FOR_MUTEX_TIME_OUT )
+            , _location( "" )
+            , _device_usb_spec( usb3_type )
+            , _power_counter( 0 )
         {
             if (!is_connected(info))
             {
@@ -884,7 +916,6 @@ namespace librealsense
             //enable reader
             CHECK_HR(MFCreateSourceReaderFromMediaSource(_source, _reader_attrs, &_reader));
             CHECK_HR(_reader->SetStreamSelection(static_cast<DWORD>(MF_SOURCE_READER_ALL_STREAMS), TRUE));
-            _power_state = D0;
 
             for( auto && xu : _xus )
                 init_xu( xu );
@@ -899,7 +930,6 @@ namespace librealsense
             safe_release(_source);
             for (auto& elem : _streams)
                 elem.callback = nullptr;
-            _power_state = D3;
         }
 
         void wmf_uvc_device::foreach_profile(std::function<void(const mf_profile& profile, CComPtr<IMFMediaType> media_type, bool& quit)> action) const
@@ -1210,5 +1240,14 @@ namespace librealsense
             _profiles.clear();
             _frame_callbacks.clear();
         }
-    }
-}
+
+        power_state wmf_uvc_device::get_power_state() const
+        {
+            LOG_ERROR( "wmf_uvc_device::get_power_state start. this = " << this );
+            std::lock_guard< std::recursive_mutex > lock( _source_lock );
+            std::string tmp = _source ? "D0" : "D3";
+            LOG_ERROR( "wmf_uvc_device::get_power_state got lock. power state is " << tmp );
+            return _source ? D0 : D3;
+        }
+    } //namespace platform
+} //namespace librealsense

src/mf/mf-uvc.h

@@ -73,7 +73,7 @@ namespace librealsense
             void stop_callbacks() override;
             void close(stream_profile profile) override;
             void set_power_state(power_state state) override;
-            power_state get_power_state() const override { return _power_state; }
+            power_state get_power_state() const override;
             std::vector<stream_profile> get_profiles() const override;
 
             static bool is_connected(const uvc_device_info& info);
@@ -119,7 +119,6 @@ namespace librealsense
             std::shared_ptr<const wmf_backend>      _backend;
 
             const uvc_device_info                   _info;
-            power_state                             _power_state = D3;
 
             CComPtr<IMFSourceReader>                _reader = nullptr;
             CComPtr<IMFMediaSource>                 _source = nullptr;
@@ -138,6 +137,7 @@ namespace librealsense
             std::vector<profile_and_callback>       _streams;
             std::mutex                              _streams_mutex;
 
+            mutable std::recursive_mutex            _source_lock; // Guarding access to _source
             named_mutex                             _systemwide_lock;
             std::string                             _location;
             usb_spec                                _device_usb_spec;
@@ -147,6 +147,7 @@ namespace librealsense
             bool                                    _streaming = false;
             std::atomic<bool>                       _is_started = false;
             std::wstring                            _device_id;
+            std::atomic< int >                      _power_counter;
             std::vector< platform::extension_unit > _xus;
         };
 

src/uvc-sensor.cpp

@@ -31,7 +31,6 @@ uvc_sensor::uvc_sensor( std::string const & name,
                         device * dev )
     : super( name, dev )
     , _device( std::move( uvc_device ) )
-    , _user_count( 0 )
     , _timestamp_reader( std::move( timestamp_reader ) )
     , _gyro_counter(0)
     , _accel_counter(0)
@@ -413,47 +412,35 @@ void uvc_sensor::reset_streaming()
 
 void uvc_sensor::acquire_power()
 {
-    std::lock_guard< std::mutex > lock( _power_lock );
-    if( _user_count.fetch_add( 1 ) == 0 )
+    try
     {
-        try
-        {
-            _device->set_power_state( platform::D0 );
-        }
-        catch( std::exception const & e )
-        {
-            _user_count.fetch_add( -1 );
-            LOG_ERROR( "acquire_power failed: " << e.what() );
-            throw;
-        }
-        catch( ... )
-        {
-            _user_count.fetch_add( -1 );
-            LOG_ERROR( "acquire_power failed" );
-            throw;
-        }
+        _device->set_power_state( platform::D0 );
+    }
+    catch( std::exception const & e )
+    {
+        LOG_ERROR( "acquire_power failed: " << e.what() );
+        throw;
+    }
+    catch( ... )
+    {
+        LOG_ERROR( "acquire_power failed" );
+        throw;
     }
 }
 
 void uvc_sensor::release_power()
 {
-    std::lock_guard< std::mutex > lock( _power_lock );
-    if( _user_count.fetch_add( -1 ) == 1 )
+    try
     {
-        try
-        {
-            _device->set_power_state( platform::D3 );
-        }
-        catch( std::exception const & e )
-        {
-            // TODO may need to change the user-count?
-            LOG_ERROR( "release_power failed: " << e.what() );
-        }
-        catch( ... )
-        {
-            // TODO may need to change the user-count?
-            LOG_ERROR( "release_power failed" );
-        }
+        _device->set_power_state( platform::D3 );
+    }
+    catch( std::exception const & e )
+    {
+        LOG_ERROR( "release_power failed: " << e.what() );
+    }
+    catch( ... )
+    {
+        LOG_ERROR( "release_power failed" );
     }
 }
 

src/uvc-sensor.h

@@ -101,8 +101,6 @@ class uvc_sensor : public raw_sensor_base
 
     std::shared_ptr< platform::uvc_device > _device;
     std::vector< platform::stream_profile > _internal_config;
-    std::atomic< int > _user_count;
-    std::mutex _power_lock;
     std::mutex _configure_lock;
     std::unique_ptr< power > _power;
     std::unique_ptr< frame_timestamp_reader > _timestamp_reader;

src/uvc/uvc-device.cpp

@@ -79,11 +79,12 @@ namespace librealsense
             return nullptr;
         }
 
-        rs_uvc_device::rs_uvc_device(const rs_usb_device& usb_device, const uvc_device_info &info, uint8_t usb_request_count) :
-                _usb_device(usb_device),
-                _info(info),
-                _action_dispatcher(10),
-                _usb_request_count(usb_request_count)
+        rs_uvc_device::rs_uvc_device( const rs_usb_device & usb_device, const uvc_device_info & info, uint8_t usb_request_count )
+            : _info( info )
+            , _power_counter( 0 )
+            , _usb_device( usb_device )
+            , _usb_request_count( usb_request_count )
+            , _action_dispatcher( 10 )
         {
             _parser = std::make_shared<uvc_parser>(usb_device, info);
             _action_dispatcher.start();
@@ -174,28 +175,53 @@ namespace librealsense
             {
                 if(state != _power_state)
                 {
+                    std::lock_guard< std::recursive_mutex > lock( _power_lock );
                     switch(state)
                     {
                         case D0:
-                            _messenger = _usb_device->open(static_cast<uint8_t>(_info.mi));
-                            if (_messenger)
+                            if( _power_counter.fetch_add( 1 ) == 0 )
                             {
-                                try{
-                                    listen_to_interrupts();
-                                } catch(const std::exception& exception) {
-                                    // this exception catching avoids crash when disconnecting 2 devices at once - bug seen in android os
-                                    LOG_WARNING("rs_uvc_device exception in listen_to_interrupts method: " << exception.what());
+                                try
+                                {
+                                    _messenger = _usb_device->open(static_cast<uint8_t>(_info.mi));
+                                    if (_messenger)
+                                    {
+                                        try
+                                        {
+                                            listen_to_interrupts();
+                                        }
+                                        catch(const std::exception& exception)
+                                        {
+                                            // this exception catching avoids crash when disconnecting 2 devices at once - bug seen in android os
+                                            LOG_WARNING("rs_uvc_device exception in listen_to_interrupts method: " << exception.what());
+                                        }
+                                        _power_state = D0;
+                                    }
+                                }
+                                //In case of failure need to decrease use counter
+                                catch( std::exception const & e )
+                                {
+                                    _power_counter.fetch_add( -1 );
+                                    throw e;
+                                }
+                                catch( ... )
+                                {
+                                    _power_counter.fetch_add( -1 );
+                                    throw;
                                 }
-                                _power_state = D0;
                             }
+                            
                             break;
                         case D3:
-                            if(_messenger)
+                            if( _power_counter.fetch_add( -1 ) == 1 )
                             {
-                                close_uvc_device();                            
-                                _messenger.reset();
+                                if( _messenger )
+                                {
+                                    close_uvc_device();
+                                    _messenger.reset();
+                                }
+                                _power_state = D3;
                             }
-                            _power_state = D3;
                             break;
                     }
                 }

src/uvc/uvc-device.h

@@ -96,7 +96,8 @@ namespace librealsense
 
             const uvc_device_info                   _info;
             power_state                             _power_state = D3; // power state change is unsupported
-
+            std::recursive_mutex                    _power_lock;
+            std::atomic< int >                      _power_counter;
             std::vector<profile_and_callback>       _streams;
 
             std::string                             _location;