Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NUI.AIAvatar] Complete refactor for enhanced modularity and customiz… #6487

Closed
wants to merge 198 commits into from

Conversation

AnglerLee
Copy link
Collaborator

Description of Change

  • Reorganized entire project structure for improved clarity and maintainability.
  • Introduced customizable interfaces for AI functionalities, allowing seamless integration of third-party modules.
  • Optimized existing code base for increased efficiency and reduced redundancy.

Seoyeon2Kim and others added 30 commits July 18, 2024 14:32
- NUI Autofill code was not removed together with dali ones.
- Now, all related codes should be deleted because there's no use case
anywhere and they don't have their internal implementation.

Signed-off-by: Seoyeon Kim <seoyeon2.kim@samsung.com>
- `CSharp_Dali_ViewImpl_AccessibilityActivate` was removed in 2018 lol
 However, at that time, NUI codes couldn't be removed together (Not sure)

- The reason `ViewImpl_AccessibilityActivate` was removed is
   `Remove functions which using internal apis

    Some binding functions use internal apis.
    It caused error when dlopen csharp binder.`

- User can use Activated event with `View.AccessibilityActivated`

Signed-off-by: Seoyeon Kim <seoyeon2.kim@samsung.com>
* [WebRTC] Change state restriction
Let we support to BoxShadow and ColorVisual, to ignore rendering inside of
View area.

None is default, same as previous logics.
CutoutView is cutout render area as bounding box of view. It is faster than CutoutViewWithCornerRadius.
CutoutWithCornerRadius is cutout render area consider the corner radius of view.

Required dali patch :
https://review.tizen.org/gerrit/c/platform/core/uifw/dali-toolkit/+/314255

Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
- Make intention revealing names for Some variables.
 Choosing names that reveal intent can make it much easier to understand.

Signed-off-by: Seoyeon Kim <seoyeon2.kim@samsung.com>
System.Reflection.Assembly.CreateInstance() can return null value.

Signed-off-by: Sangyoon Jang <jeremy.jang@samsung.com>
This patch changes internal tensor size limit in MachineLearning.Inference.
Tizen Native ML api's max tensor size has been changed to 256.

Signed-off-by: Yelin Jeong <yelini.jeong@samsung.com>
Signed-off-by: Seungho Baek <sbsh.baek@samsung.com>
Since some useless animation was running internally until GC comes,
CPU usage might be increased for this sample.

To avoid useless rendering, let we ensurely dispose not-using animation.

Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
Make we use unified code style when we connect/disconnect native signal
to C# EventHandler.

It will be reduce some copy-paste the non-common style implementation.

This PR will change below classes

* (internal)Application and it's family
* (internal)ObjectRegistry
* (internal)Builder
* (internal)GaussianBlurView
* ImageView and it's family
* View, ViewAccessibility
* Clipboard
* Capture

Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
Signed-off-by: Seungho Baek <sbsh.baek@samsung.com>
Co-authored-by: Woochan Lee <wc0917.lee@samsung.com>
…view

Until now, VisibilityChangedEventArgs.View return sender itself.

It is useless information for user.

Follow up some other platform's behavior,
( reference : https://developer.android.com/reference/android/view/View#onVisibilityChanged(android.view.View,%20int) )
Let we make VisibilityChangedEventArgs.View return visibility changed view.

Relative dali patch :

- https://review.tizen.org/gerrit/c/platform/core/uifw/dali-core/+/315475
- https://review.tizen.org/gerrit/c/platform/core/uifw/dali-csharp-binder/+/315484

Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
Found some build warning list by below command

dotnet build /p:BuildWithAnalyzer=True -consoleloggerparameters:NoSummary

Their are too many warnings occurs, let we remove them by multiple phases.

Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
- TCT crash issue, log =>
E/STDERR  (21795): Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
E/STDERR  (21795):    at Tizen.NUI.BaseComponents.View.Dispose(DisposeTypes type) in Tizen.NUI.dll:token 0x6002b7c+0x591
E/STDERR  (21795):    at Tizen.NUI.BaseHandle.Dispose() in Tizen.NUI.dll:token 0x6000e5c+0xc
E/STDERR  (21795):    at Tizen.NUI.DisposeQueue.ProcessDisposablesIncrementally() in Tizen.NUI.dll:token 0x6000238+0x79
E/STDERR  (21795):    at Tizen.NUI.DisposeQueue.ProcessDisposables() in Tizen.NUI.dll:token 0x6000237+0xd5
E/STDERR  (21795): onSigabrt called
Apply some comments that report at PR #6079

- `Color.A` getter apply `Opacity` changeness
- Make `VisualFittingMode` have no effect for `ColorVisual` and `BorderVisual`
- `GetVisualProperty` return valid value after call `GetCurrentVisualProperty`
- Make `VisualBase` class as abstract. Let we don't allow to call this class constructor.

Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
Add SynchrousSizing property - reload as size of visual.

It will be useful when we want to load image as specific size of view,
so the memory is reduced.

Signed-off-by: Eunki Hong <eunkiki.hong@samsung.com>
Signed-off-by: Seoyeon Kim <seoyeon2.kim@samsung.com>
Requests asynchronous rendering of text with a fixed size.

Signed-off-by: Bowon Ryu <bowon.ryu@samsung.com>
@TizenAPI-Bot
Copy link
Collaborator

Public API Changed

Please follow the ACR process for the changed API below.
Added: 236, Removed: 0, Changed: 0

Internal API Changed

Added: 14, Removed: 199, Changed: 0

Copy link
Contributor

@hinohie hinohie left a comment

Choose a reason for hiding this comment

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

Please add Hidden tag before open ACR.

<ProjectReference Include="..\Tizen\Tizen.csproj" />
<ProjectReference Include="..\Tizen.Log\Tizen.Log.csproj" />
<ProjectReference Include="..\Tizen.Uix.Tts\Tizen.Uix.Tts.csproj" />
<ProjectReference Include="..\Tizen.NUI\Tizen.NUI.csproj" />
<ProjectReference Include="..\Tizen.NUI.Scene3D\Tizen.NUI.Scene3D.csproj" />
<ProjectReference Include="..\Tizen.MachineLearning.Inference\Tizen.MachineLearning.Inference.csproj" />
<ProjectReference Include="..\Tizen.MachineLearning.Inference\Tizen.MachineLearning.Inference.csproj" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<ProjectReference Include="..\Tizen.MachineLearning.Inference\Tizen.MachineLearning.Inference.csproj" />
<ProjectReference Include="..\Tizen.MachineLearning.Inference\Tizen.MachineLearning.Inference.csproj" />

Minor. Remove this tab

Copy link
Collaborator Author

@AnglerLee AnglerLee Dec 3, 2024

Choose a reason for hiding this comment

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

Thank you. I will modify the code.

@dongsug-song
Copy link
Contributor

@AnglerLee
아래와 같은 Bot comment 가 있습니다.
이것은 public api 가 추가,삭제,변경 되었을때 ACR 이 필요해서 뜨는 것입니다.

Please follow the ACR process for the changed API below.
Added: 236, Removed: 0, Changed: 0

TCSACR 을 진행하셔야 될 것 같습니다.
예시) https://jira.sec.samsung.net/browse/TCSACR-586

@jaehyun0cho
@Seoyeon2Kim
@taehyub
@lwc0917
안녕하세요.
패치가 큰데요, 리뷰 부탁드립니다.

@AnglerLee
Copy link
Collaborator Author

리팩토링 이후 충분한 리뷰를 거치지 못해서요.
일단 Hidden으로 처리하고 완성도를 높인 후에 ACR 진행 하도록 하겠습니다!
감사합니다.

@TizenAPI-Bot
Copy link
Collaborator

Internal API Changed

Added: 250, Removed: 199, Changed: 0

Copy link
Contributor

@Seoyeon2Kim Seoyeon2Kim left a comment

Choose a reason for hiding this comment

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

Wow this PR is very huge.
I'll read the rest soon.

/// Gets or sets the size of the current audio data chunk in bytes.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public int AudioBytes { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Just my curiosity :
I'm not familiar with AI Avatar actually, so could you explain why this type is int, not byte array like the above (AudioData) please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The variables AudioBytes, TotalBytes, and ProcessedBytes are all declared as 'int' because they represent the size of processed audio data. The actual audio data is stored in a variable named AudioData which is declared as a byte array.

using System.Text;
using System.Threading.Tasks;

namespace Tizen.AIAvatar.Samsung
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor.
@dongsug-song , is it okay to add Samsung suffix for TizenFX coding convention?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I apologize for the mistake in adding a suffix. I will correct it right away.

model = "tts-1",
input = text,
voice = voice ?? "alloy",
response_format = "pcm", // PCM 포맷으로 변경
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be this Korean comment not be updated yet?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will remove the comments for clean code. I'll make the changes. 😄

@TizenAPI-Bot
Copy link
Collaborator

Internal API Changed

Added: 259, Removed: 199, Changed: 0

…enaming them accordingly for better code organization and readability.
@TizenAPI-Bot
Copy link
Collaborator

Internal API Changed

Added: 263, Removed: 199, Changed: 0

…internal variables, so I changed the values of the structure to public.
@AnglerLee
Copy link
Collaborator Author

I have documented the implementation of AIAvatar refactoring. Please refer to the following for review:
[Implementation] (https://confluence.sec.samsung.net/pages/viewpage.action?pageId=975063819)
[Sample & Test] (https://confluence.sec.samsung.net/pages/viewpage.action?pageId=975063825)

@TizenAPI-Bot
Copy link
Collaborator

Internal API Changed

Added: 263, Removed: 199, Changed: 0

@dongsug-song
Copy link
Contributor

@AnglerLee
Hello,
The TizenFX main branch has been changed for API13 (tizen_10).
So the DevelNUI branch also did a hard reset with the master branch to sync this.
I think you need to rebase this PR or re-upload the new PR.
I am sorry for the inconvenience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[AIAvatar] Newton Json dependency